feat(aztec-nr): Compute unconstrained tag in Noir over PXE and generalize get_next_tagging_index oracle#23796
Conversation
This PR bumped ORACLE_VERSION_MAJOR from 27 to 28 in aztec-nr/aztec/src/oracle/version.nr and yarn-project/pxe/src/oracle_version.ts, but missed the duplicate constant in the parallel sublib that protocol contracts depend on (noir-contracts/contracts/protocol/ aztec_sublib/src/oracle/version.nr). Protocol contracts were therefore still being compiled with major=27, and PXE rejected them at runtime with 'Incompatible private environment version: expected oracle major version 28, got 27', breaking every e2e test that loads a protocol contract.
nchamo
left a comment
There was a problem hiding this comment.
Should we be merging noir-projects/noir-contracts/pinned-standard-contracts.tar.gz? Or was that an accident?
| get_next_app_tag_as_sender(sender, recipient) | ||
| let secret = get_app_tagging_secret(sender, recipient); | ||
| let index = get_next_tagging_index(secret, ONCHAIN_UNCONSTRAINED); | ||
| poseidon2_hash([secret, index as Field]) |
There was a problem hiding this comment.
We don't need to do it in this PR, but I'm curious about how this will change we start supporting constrained
| /// This is the secret half of unconstrained-delivery tag derivation. The simulator performs the ECDH multiply - | ||
| /// which needs the sender's master incoming viewing secret key and so can only run in the PXE - siloes the result | ||
| /// to the calling contract, and returns it. The caller finishes the tag by obtaining an index from | ||
| /// [`get_next_tagging_index`] and hashing the two together. Handshake-origin sends skip this entirely: the registry | ||
| /// provides the secret instead. |
There was a problem hiding this comment.
I don't like this doc very much. "the secret half" is not clear enough for me. The ECDH multiply is to detail oriented. Actually, I think it would we best if we don't mention ECDH here, since maybe there is another way the secret was originated
I think we could rename the oracle to reflect that this is a tagging secret that is not calculated by contracts. It's hold or calculated somewhere else, and this oracle is meant to retrieve it. That's the diff between using this oracle and the secrets we calculate in contracts (like handshakes): the origin
Am I making sense? Do you see this differently?
There was a problem hiding this comment.
I'm having doubts about my previous comment. So feel free to ignore it or share your opinion, I think I mixed things
There was a problem hiding this comment.
The comment is still a little unclear either way as it is too much in the internals. I will edit it.
There was a problem hiding this comment.
I edited the comments to be more general and I think they are clearer now.
No issue, fixes confusing CI errors I ran into on #23796 when the `aztec_sublib` oracle version was not updated http://ci.aztec-labs.com/83a883aee887f12c: <details><summary>Details</summary> <p> ``` 17:46:29 17:46:29 ● e2e_blacklist_token_contract transfer public › failure cases › transfer from a blacklisted account 17:46:29 17:46:29 TypeError: Cannot read properties of undefined (reading 'check') 17:46:29 17:46:29 22 | 17:46:29 23 | afterEach(async () => { 17:46:29 > 24 | await t.tokenSim.check(); 17:46:29 | ^ 17:46:29 25 | }); 17:46:29 26 | 17:46:29 27 | it('transfer less than balance', async () => { 17:46:29 17:46:29 at Object.check (e2e_blacklist_token_contract/transfer_public.test.ts:24:22) 17:46:29 17:46:29 ● e2e_blacklist_token_contract transfer public › failure cases › transfer to a blacklisted account 17:46:29 17:46:29 Simulation error: Incompatible private environment version: The contract was compiled with an older version of Aztec.nr than your private environment supports. Recompile the contract with a compatible version of Aztec.nr. See https://docs.aztec.network/errors/8 (expected oracle major version 28, got 27) 17:46:29 17:46:29 at ContractClassRegistry.publish 17:46:29 at MultiCallEntrypoint.entrypoint 17:46:29 17:46:29 Cause: 17:46:29 Incompatible private environment version: The contract was compiled with an older version of Aztec.nr than your private environment supports. Recompile the contract with a compatible version of Aztec.nr. See https://docs.aztec.network/errors/8 (expected oracle major version 28, got 27) 17:46:29 17:46:29 85 | if (major !== ORACLE_VERSION_MAJOR) { 17:46:29 86 | const hint = major > ORACLE_VERSION_MAJOR ? 'The contract was compiled with a newer version of Aztec.nr than your private environment supports. Upgrade your private environment to a compatible version.' : 'The contract was compiled with an older version of Aztec.nr than your private environment supports. Recompile the contract with a compatible version of Aztec.nr.'; 17:46:29 > 87 | throw new Error(`Incompatible private environment version: ${hint} See https://docs.aztec.network/errors/8 (expected oracle major version ${ORACLE_VERSION_MAJOR}, got ${major})`); 17:46:29 | ^ 17:46:29 88 | } 17:46:29 89 | this.contractOracleVersion = { 17:46:29 90 | major, 17:46:29 17:46:29 at PrivateExecutionOracle.assertCompatibleOracleVersion (../../pxe/dest/contract_function_simulator/oracle/utility_execution_oracle.js:87:19) ``` </p> </details> We are told there is an oracle mismatch, but there is nothing to indicate it is in the aztec-nr sublib as I had already updated the actual aztec-nr version constant. The error also only comes about at runtime in an e2e test. We should be able to error out in the pre-existing `check_oracle_version` scripts for pxe/txe.
…e_execution_oracle.ts Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>
…e_execution_oracle.ts Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>
Looks to be intended. The tarball is an existing pinned artifact, and looks to be getting refreshed along with the other standard contracts after the oracle version change. |
Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>
Co-authored-by: Nicolas Chamo <nicolas@chamo.com.ar>
BEGIN_COMMIT_OVERRIDE chore: document browser kv-store backend migration (AztecProtocol#23779) feat(aztec-nr): Compute unconstrained tag in Noir over PXE and generalize get_next_tagging_index oracle (AztecProtocol#23796) refactor(txe): migrate rpc_translator to typed oracle registry (AztecProtocol#23530) feat(pxe)!: allow apps to inject tagging secrets into getPendingTaggedLogs (AztecProtocol#23777) feat(txe): add oracle roundtrip test framework (AztecProtocol#23537) fix: embedded wallet defaults to proposed (AztecProtocol#23819) feat(aztec-nr): discover non-interactive handshakes in the registry (AztecProtocol#23806) refactor(txe): normalize deploy and addAccount to oracle registry (AztecProtocol#23536) fix: wait for checkpoint during sandbox setup (AztecProtocol#23834) feat(standard-contracts): graduate handshake registry to standard contract (AztecProtocol#23833) END_COMMIT_OVERRIDE
Fixes F-695
Fixes F-696
Fixes F-513
get_app_tagging_secretoracle rather thanget_next_app_tag_as_senderand computing the tag in the PXEget_next_constrained_tagging_index->get_next_tagging_index(secret, mode)