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

The _public_ kernel circuit should inject a nonce into new note hashes #1386

Closed
Tracked by #2783
iAmMichaelConnor opened this issue Aug 2, 2023 · 4 comments · Fixed by #7715
Closed
Tracked by #2783

The _public_ kernel circuit should inject a nonce into new note hashes #1386

iAmMichaelConnor opened this issue Aug 2, 2023 · 4 comments · Fixed by #7715
Assignees
Labels
T-fix Type: fix this. Not quite a bug.

Comments

@iAmMichaelConnor
Copy link
Contributor

We're not-yet injecting nonces into commitments which have been created by public functions.
@dbanks12 is currently working on doing this for private functions.

When a public function wants to insert a new note hash into the utxo tree, the public kernel circuit should inject a nonce (in the same way as is done for private functions).

  • Public Function creates inner_note_hash with some preimage.
    • If the dapp knows in advance what the preimage will be (i.e. if the note's preimage is not derived at runtime) it can hold on to that preimage for later.
    • If the dapp doesn't know in advance what the preimage will be (because some of its values are derived at runtime), it will need to hold on to the info that it has. The rest of the preimage will need to be communicated to the dapp, e.g. as an unencrypted log, or by storing the preimage in public state (emitting as an unencrypted log would be preferable).
    • In both the above cases, the dapp can be given the tx_hash of the tx.
  • The dapp can listen for the tx_hash, by subscribing to an event or polling. (This is a separate TODO!!!)
  • The dapp might also need to listen for an explicit event topic (This is a separate TODO!!!)
  • Once the tx_hash is seen, the accompanying L1 calldata for this tx will include:
    • nullifiers
    • new unique_siloed_note_hashes
    • info about public state transitions
    • logs
    • contract_address (known because this is a public function that's been executed)
  • Given the tx_hash, logs, and already-known preimage data, the dapp can learn the nonce of this commitment:
    • Iterate through each note hash:
      brute_force_derive_nonce(...) {
          let preimage = ... // somehow assemble the preimage of the `inner_note_hash` from `log` and already-known preimage data.
          let target_inner_note_hash = h(...preimage);
          let target_siloed_note_hash = h(inner_note_hash, contract_address);
          for (unique_siloed_note_hash, i) in unique_siloed_note_hashes {
              let trial_nonce = h(nullifiers[0], i);
              let trial_siloed_note_hash = h(target_siloed_note_hash, trial_nonce);
              if (trial_siloed_note_hash === unique_siloed_note_hash) {
                  // match found!!!
                  return trial_nonce;
              }
          }
      }

Brute-forcing doesn't seem too terrible. If you already know the tx_hash, you'll only be trialling as many commitments as can be created by a single tx.

@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 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 this to the 📢 Initial Public Sandbox Release milestone Aug 25, 2023
@dbanks12
Copy link
Contributor

dbanks12 commented Sep 1, 2023

I believe this is blocked by #1420

@dbanks12 dbanks12 self-assigned this Sep 5, 2023
@dbanks12 dbanks12 removed this from the 📢 Initial Public Sandbox Release milestone Sep 5, 2023
@rahul-kothari
Copy link
Contributor

also referenced in #4891

@benesjan
Copy link
Contributor

@dbanks12 is this issue still planned to be tackled? I just stumbled upon it because it would allow us to decrease gate counts (see here). I could help tackling it if you don't have enough time to do that.

@iAmMichaelConnor
Copy link
Contributor Author

I still think it needs to be tackled, and I reckon your team (Jan) is now perfectly placed to tackle it (whereas it'd be a big context-switch for david) :)

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-fix Type: fix this. Not quite a bug.
Projects
Archived in project
4 participants