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

Do merkle membership checks when consuming L1->L2 messages #1383

Closed
iAmMichaelConnor opened this issue Aug 2, 2023 · 0 comments · Fixed by #4141
Closed

Do merkle membership checks when consuming L1->L2 messages #1383

iAmMichaelConnor opened this issue Aug 2, 2023 · 0 comments · Fixed by #4141
Labels
A-security Area: Relates to security. Something is insecure. T-fix Type: fix this. Not quite a bug.

Comments

@iAmMichaelConnor
Copy link
Contributor

These checks aren't being done yet.

@iAmMichaelConnor iAmMichaelConnor added the T-fix Type: fix this. Not quite a bug. label Aug 2, 2023
@github-project-automation github-project-automation bot moved this to Todo in A3 Aug 2, 2023
@iAmMichaelConnor iAmMichaelConnor added this to the 📥 L1->L2 Calls milestone Aug 2, 2023
iAmMichaelConnor pushed a commit that referenced this issue Aug 3, 2023
# Description

The way nonces work now, there can be inconsistencies in nonce
assignment in the simulator vs the private kernel. Furthermore, you
cannot know during function execution what the full set of commitments
will be for the whole TX as some new commitments may be nullified and
squashed. But we still want the ability to determine nonces and
therefore uniqueNoteHashes from L1 calldata alone. I am sure I am not
explaining all of the issues well enough, but it was determined that the
current nonce paradigm will not work and therefore we must rework it.

Rework nonces so that siloing by contract address happens first and
uniqueness comes later. For now, nonces are injeced by the private
ordering circuit (vs suggestion which was base rollup circuit). Pending
notes and their reads have no nonces when processed in kernel. The
public kernel (and therefore all commitments created in public
functions) does not use nonces.

Here was Mike's proposal for the rework:

![image](https://github.com/AztecProtocol/aztec-packages/assets/47112877/7b20c886-1e92-452c-a886-c3da5ed64e17)

Why not just use leaf index as nonce?

![image](https://github.com/AztecProtocol/aztec-packages/assets/47112877/e6337107-ac93-4a3b-b83c-27213cb5133d)

## Followup tasks
* #1029
* #1194
* #1329
* #1407
* #1408
* #1409
* #1410
* Future enhancement: The root rollup circuit could insert all messages
at the very beginning of the root rollup circuit, so that txs within the
rollup can refer to that state root and read L1>L2 messages immediately.
* #1383
* #1386
* We should implement subscription / polling methods for Aztec logs
* We should maybe write rpc functions which allow calldata to be
subscribed-to, keyed by tx_hash.
* If a dapp wants to write a note from a public function, a lot of honus
will be on a dapp developer to retain preimage information, query the
blockchain, and derive the nonce. We should provide some examples to
demonstrate this pattern.
AztecBot pushed a commit to AztecProtocol/docs that referenced this issue Aug 3, 2023
# Description

The way nonces work now, there can be inconsistencies in nonce
assignment in the simulator vs the private kernel. Furthermore, you
cannot know during function execution what the full set of commitments
will be for the whole TX as some new commitments may be nullified and
squashed. But we still want the ability to determine nonces and
therefore uniqueNoteHashes from L1 calldata alone. I am sure I am not
explaining all of the issues well enough, but it was determined that the
current nonce paradigm will not work and therefore we must rework it.

Rework nonces so that siloing by contract address happens first and
uniqueness comes later. For now, nonces are injeced by the private
ordering circuit (vs suggestion which was base rollup circuit). Pending
notes and their reads have no nonces when processed in kernel. The
public kernel (and therefore all commitments created in public
functions) does not use nonces.

Here was Mike's proposal for the rework:

![image](https://github.com/AztecProtocol/aztec-packages/assets/47112877/7b20c886-1e92-452c-a886-c3da5ed64e17)

Why not just use leaf index as nonce?

![image](https://github.com/AztecProtocol/aztec-packages/assets/47112877/e6337107-ac93-4a3b-b83c-27213cb5133d)

## Followup tasks
* AztecProtocol/aztec-packages#1029
* AztecProtocol/aztec-packages#1194
* AztecProtocol/aztec-packages#1329
* AztecProtocol/aztec-packages#1407
* AztecProtocol/aztec-packages#1408
* AztecProtocol/aztec-packages#1409
* AztecProtocol/aztec-packages#1410
* Future enhancement: The root rollup circuit could insert all messages
at the very beginning of the root rollup circuit, so that txs within the
rollup can refer to that state root and read L1>L2 messages immediately.
* AztecProtocol/aztec-packages#1383
* AztecProtocol/aztec-packages#1386
* We should implement subscription / polling methods for Aztec logs
* We should maybe write rpc functions which allow calldata to be
subscribed-to, keyed by tx_hash.
* If a dapp wants to write a note from a public function, a lot of honus
will be on a dapp developer to retain preimage information, query the
blockchain, and derive the nonce. We should provide some examples to
demonstrate this pattern.
@iAmMichaelConnor iAmMichaelConnor added the A-security Area: Relates to security. Something is insecure. label Aug 25, 2023
@iAmMichaelConnor iAmMichaelConnor removed this from the 📥 L1->L2 Calls milestone Aug 27, 2023
LHerskind added a commit that referenced this issue Jan 24, 2024
Fixes #1383
- Adding more tests
- Adding assertion strings 
- Remove `root` from oracle response as it is not needed
@github-project-automation github-project-automation bot moved this from Todo to Done in A3 Jan 24, 2024
AztecBot pushed a commit to AztecProtocol/aztec-nr that referenced this issue Jan 25, 2024
Fixes AztecProtocol/aztec-packages#1383
- Adding more tests
- Adding assertion strings 
- Remove `root` from oracle response as it is not needed
michaelelliot pushed a commit to Swoir/noir_rs that referenced this issue Feb 28, 2024
Fixes AztecProtocol#1383
- Adding more tests
- Adding assertion strings 
- Remove `root` from oracle response as it is not needed
superstar0402 added a commit to superstar0402/aztec-nr that referenced this issue Aug 16, 2024
# Description

The way nonces work now, there can be inconsistencies in nonce
assignment in the simulator vs the private kernel. Furthermore, you
cannot know during function execution what the full set of commitments
will be for the whole TX as some new commitments may be nullified and
squashed. But we still want the ability to determine nonces and
therefore uniqueNoteHashes from L1 calldata alone. I am sure I am not
explaining all of the issues well enough, but it was determined that the
current nonce paradigm will not work and therefore we must rework it.

Rework nonces so that siloing by contract address happens first and
uniqueness comes later. For now, nonces are injeced by the private
ordering circuit (vs suggestion which was base rollup circuit). Pending
notes and their reads have no nonces when processed in kernel. The
public kernel (and therefore all commitments created in public
functions) does not use nonces.

Here was Mike's proposal for the rework:

![image](https://github.com/AztecProtocol/aztec-packages/assets/47112877/7b20c886-1e92-452c-a886-c3da5ed64e17)

Why not just use leaf index as nonce?

![image](https://github.com/AztecProtocol/aztec-packages/assets/47112877/e6337107-ac93-4a3b-b83c-27213cb5133d)

## Followup tasks
* AztecProtocol/aztec-packages#1029
* AztecProtocol/aztec-packages#1194
* AztecProtocol/aztec-packages#1329
* AztecProtocol/aztec-packages#1407
* AztecProtocol/aztec-packages#1408
* AztecProtocol/aztec-packages#1409
* AztecProtocol/aztec-packages#1410
* Future enhancement: The root rollup circuit could insert all messages
at the very beginning of the root rollup circuit, so that txs within the
rollup can refer to that state root and read L1>L2 messages immediately.
* AztecProtocol/aztec-packages#1383
* AztecProtocol/aztec-packages#1386
* We should implement subscription / polling methods for Aztec logs
* We should maybe write rpc functions which allow calldata to be
subscribed-to, keyed by tx_hash.
* If a dapp wants to write a note from a public function, a lot of honus
will be on a dapp developer to retain preimage information, query the
blockchain, and derive the nonce. We should provide some examples to
demonstrate this pattern.
superstar0402 added a commit to superstar0402/aztec-nr that referenced this issue Aug 16, 2024
Fixes AztecProtocol/aztec-packages#1383
- Adding more tests
- Adding assertion strings 
- Remove `root` from oracle response as it is not needed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-security Area: Relates to security. Something is insecure. T-fix Type: fix this. Not quite a bug.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant