Skip to content

feat: add designated caller on token portal and update uniswap to use it.#896

Merged
rahul-kothari merged 3 commits into
masterfrom
rk/designated_caller
Jun 22, 2023
Merged

feat: add designated caller on token portal and update uniswap to use it.#896
rahul-kothari merged 3 commits into
masterfrom
rk/designated_caller

Conversation

@rahul-kothari

@rahul-kothari rahul-kothari commented Jun 21, 2023

Copy link
Copy Markdown
Contributor

Description

Fix #877 -> currently, anyone can call our token portal to consume the withdraw message. This doesn't seem a problem there is no use of msg.sender in its code and the funds would go to the recipient specified in the message hash. However, in some cases it might be important, like when a contract is consuming multiple messages as part of a smart contract function - in such a case only this contract should call it.

Note - once again we implement this at the portal and not the protocol side to maintain flexibility.

I have added tests in solidity to check that caller works as expected.

This builds on #876 which I will fix after this pr! There I will add more tests on an e2e scale.

Checklist:

  • I have reviewed my diff in github, line by line.
  • Every change is related to the PR description.
  • I have linked this pull request to the issue(s) that it resolves.
  • There are no unexpected formatting changes, superfluous debug logs, or commented-out code.
  • The branch has been merged or rebased against the head of its merge target.
  • I'm happy for the PR to be merged at the reviewer's next convenience.

@rahul-kothari rahul-kothari force-pushed the rk/designated_caller branch 2 times, most recently from 4e68618 to 7500298 Compare June 21, 2023 12:10
@rahul-kothari rahul-kothari marked this pull request as ready for review June 21, 2023 13:32
@rahul-kothari rahul-kothari requested a review from LHerskind June 21, 2023 13:32
Comment thread l1-contracts/test/portals/TokenPortal.sol Outdated
Comment thread l1-contracts/test/portals/TokenPortal.t.sol Outdated
Comment thread l1-contracts/test/portals/TokenPortal.t.sol Outdated
Comment thread yarn-project/end-to-end/src/cross_chain/test_harness.ts Outdated
Comment thread yarn-project/noir-contracts/src/contracts/uniswap_contract/src/main.nr Outdated
Comment thread yarn-project/noir-contracts/src/contracts/uniswap_contract/src/main.nr Outdated
@LHerskind LHerskind changed the title add designated caller on tokenPortal.withdraw() and update accordingly feat: add designated caller on token portal and update uniswap to use it. Jun 22, 2023
@rahul-kothari rahul-kothari force-pushed the rk/designated_caller branch from 419bf43 to f758921 Compare June 22, 2023 10:36
@rahul-kothari rahul-kothari requested a review from LHerskind June 22, 2023 10:37
Comment thread l1-contracts/test/portals/TokenPortal.t.sol
Comment thread l1-contracts/test/portals/TokenPortal.t.sol
Comment thread yarn-project/noir-contracts/src/contracts/uniswap_contract/src/main.nr Outdated
@rahul-kothari rahul-kothari requested a review from LHerskind June 22, 2023 11:15
@rahul-kothari rahul-kothari enabled auto-merge (squash) June 22, 2023 11:15
@rahul-kothari rahul-kothari merged commit d482ebc into master Jun 22, 2023
@rahul-kothari rahul-kothari deleted the rk/designated_caller branch June 22, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Designated caller asset contract/portal

2 participants