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

L1 <> L2 messages need to be scoped to a particular version of Aztec #359

Closed
Tracked by #824
joeandrews opened this issue Apr 25, 2023 · 2 comments · Fixed by #1245
Closed
Tracked by #824

L1 <> L2 messages need to be scoped to a particular version of Aztec #359

joeandrews opened this issue Apr 25, 2023 · 2 comments · Fixed by #1245

Comments

@joeandrews
Copy link
Contributor

Portal contracts need to define which version of Aztec, they can consume messages from.

Example

Lets say a portal contract manages Dai deposits. A user deposits 1,000 Dai to the portal contract that adds a message to the L1 > L2 Message Box.

The rollup implementation is then upgraded to support a less secure hash function / or some other change the user / dApp developer does not agree with.

In order to protect against the message being consumed in the newer version, the message needs to be scoped to a version.

@github-project-automation github-project-automation bot moved this to Todo in A3 Apr 25, 2023
@LHerskind LHerskind moved this from Todo to Done in A3 May 22, 2023
@LHerskind
Copy link
Contributor

Currently the implementation is assuming one rollup due to the upcoming upgrade RFP this have not been changed yet, as it depends on the mechanism chosen.

ludamad pushed a commit that referenced this issue Jul 14, 2023
* lookup grand product relation tests passing

* ignore final entry in table polys for consistency chec, same reason as for selectors

* adding eta and lookup gp delta to relation params

* incorporate lookup gp init relation into tests

* correcting the degree of lookup relation
@LHerskind
Copy link
Contributor

LHerskind commented Jul 27, 2023

Idea to limit based on versioning that is enforced fully on L1 (no circuit changes).

  • At the registry:
    • add a mapping rollupToVersion that maps from an address to the specific version. This entails that a specific rollup can only be listed once, to not have multiple version with same address.
    • at upgrade ensure that rollup is not already listed, and update the version in rollupToVersion, revert if already defined.
    • add getVersionFor(address) function that returns the version for a specific address. Revert if the address is not the rollup for any version.
    • at the constructor insert a "dead" rollup to ensure that 0's is not a "catch-all". Also beneficial to start version 1 to align with rest of system.
  • For the MessageBox library,
    • the entry structure can be updated to include a version number. We can use 32 bits just fine, as it is increment we will probably be fine with 2**32-1 versions supported.
    • the insert function also check that the versions match and version != 0.
  • In the Inbox:
    • at sendL2Message, store the version number in the entry.
    • at batchConsume fetch the version for msg.sender and for every message check that entry.version matches this version,
    • Can delete onlyRollup modifier as this is implicitly checked through the version check.
  • In the Outbox:
    • at sendL1Messages, read the version using REGISTRY.getVersionFor(msg.sender) and insert into the entries
    • at consume check that entry.version matches the version of the message.
  • Note. The Rollup contract itself stores a version that it is passing on to the execution layer in the globals, but this value does not necessarily matches the version number, so invalid config can make it impossible for a rollup to send stuff to L1. Also, the listing is permissioned now, but can be altered to match other proposals.

LHerskind added a commit that referenced this issue Jul 29, 2023
# Description
Fixes #359 and #624.

# Checklist:

- [ ] I have reviewed my diff in github, line by line.
- [ ] Every change is related to the PR description.
- [ ] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
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.
LHerskind added a commit that referenced this issue Jul 30, 2023
# Description
Fixes #359 and #624.

# Checklist:

- [ ] I have reviewed my diff in github, line by line.
- [ ] Every change is related to the PR description.
- [ ] I have
[linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue)
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.
@Maddiaa0 Maddiaa0 removed this from A3 Oct 24, 2023
codygunton pushed a commit that referenced this issue Jan 23, 2024
* lookup grand product relation tests passing

* ignore final entry in table polys for consistency chec, same reason as for selectors

* adding eta and lookup gp delta to relation params

* incorporate lookup gp init relation into tests

* correcting the degree of lookup relation
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 a pull request may close this issue.

2 participants