Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ETHEREUM-CONTRACTS] | #1038 | Allow regular contracts to be the target of callAppAction #1242

Closed
wants to merge 6 commits into from

Conversation

0xdavinchee
Copy link
Contributor

Description

Motivation: To relax the requirement that callAppAction can only be used on super app contracts.

Context: This change allows all contracts to be the target of callAppAction as long as the function on the contract satisfies two requirements:

  1. The function you want to call via callAppAction must take bytes (the passed context from the host) as the last parameter.
  2. You must return the "correct" context bytes from the contract in the function, that is, untouched.

NOTE: this means that EOAs cannot be targets of callAppAction as any call against an EOA returns empty bytes.

This was achieved simply by deleting the isAppActive modifier from Superfluid.sol, ISuperfluid.sol and any associated custom errors only used by this modifer.

Issue: #1038

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

I have added tests to ensure that non-super app contracts can be the target of callAppAction and callAppActionWithContext.

  • 8.1-8.3 tests callAppAction reverting when passing invalid calldata
  • 8.4 tests callAppAction reverting when an EOA is the target, ctx is malformatted because bytes returned is "0x"
  • 8.5.0 tests a revert given requirement 1 failing
  • 8.5.1 tests a revert given requirement 1 passing, but requirement 2 failing (returning empty bytes)
  • 8.5.2 tests happy path callAppAction
  • 8.5.3 tests happy path callAppActionWithContext
  • 8.5.4 tests a revert callAppActionWithContext given that we ignore the context passed by the host via the initial callAppAction and pass in our own context

- remove isAppActive modifier from `Superfluid.sol` and `ISuperfluid.sol`
- remove `HOST_NOT_A_SUPER_APP` and `HOST_SUPER_APP_IS_JAILED` custom errors
- create new `NonSuperAppContractMock.sol` to test `callAppAction` and `callAppActionWithContext` on a non-super app contract
- Add two functions to `SuperAppMocks` to test `callAppActionWithContext` on our non-superApp contract
- modify CHANGELOG.md
- remove _ctx param comment
- change ISuperApp app => address target
- revert on callAppAction if target is host or agreement
@github-actions
Copy link

Changelog Reminder

Reminder to update the CHANGELOG.md for any of the modified packages in this PR.

  • CHANGELOG.md modified
  • Double check before merge

@hellwolf hellwolf added the Tag: Idea Raw idea, questions, thoughts and brainstorming notes label Feb 17, 2023
@0xdavinchee
Copy link
Contributor Author

closing this PR for now as this is stale and nobody is asking for it

@0xdavinchee 0xdavinchee closed this Mar 9, 2023
@hellwolf hellwolf deleted the 1038-relax-call-app-action-restriction branch August 24, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tag: Idea Raw idea, questions, thoughts and brainstorming notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants