decay interval fix #20075
Closed
decay interval fix #20075
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Linear issue: [AVM-76](https://linear.app/aztec-labs/issue/AVM-76/poseidon2)
- explicitly set timeout to be less than 6 hours so that workflow can complete network teardown - when rolling pods, wait for every state to progress before continuing, wait for PVC to be completely wiped, and roll HA db first and initialize the DB, this will reduce k8 provisioning flakes - reestablish prometheus port forwards on failure for prover-node test and wait for proven chain to update before killing prover broker to make the test more consistent - add component label to HA postgres DB
As Santiago proposed [here](#19899 (review)) makes sense to have `BlockHash` extend from `Fr` instead from `Buffer32` because the hash is the output of Poseidon2 hash and hence natively a field. Also introduced branding to avoid collisions with `Fr` and cleaned up the type from unused functions.
Remove the flag `update_fast` from the script as it didn't work. Simplified the script as a result, added an helper, added `prove_and_verify` flag to verify the validity of the pinned inputs.
BEGIN_COMMIT_OVERRIDE fix(avm)!: pre-audit poseidon2 (#19963) END_COMMIT_OVERRIDE
As Santiago proposed [here](#19899 (review)) makes sense to have `BlockHash` extend from `Fr` instead from `Buffer32` because the hash is the output of Poseidon2 hash and hence natively a field. Also introduced branding to avoid collisions with `Fr` and cleaned up the type from unused functions.
Protocol circuits are about to undergo an audit and with that also the protocol contracts. Since the Aztec.nr macros are in flux and very complicated it has been decided that we want to strip them from the protocol contracts. This has an undesirable consequence and that is that if we strip the macros we no longer get the nice Noir interface generated (e.g. `MyContract::at(address)`). To work around this problem I've created copies of the original contracts in `noir-projects/noir-contracts/contracts/protocol_interface` that keep all the function signatures but none of the implementations. With this we can use the interfaces in other Noir contracts just by updating the dependency to point to the `protocol_interface` dir instead of `protocol` dir. Note that I didn't create a copy for the `ContractClassRegistry` as that one was never used in any other contract (and I don't think there is an expectation of this ever being the case). This was also not done for the `MultiCallEntrypoint` and `PublicChecks` contracts as those are not protocol contracts even though they are in the `protocol/` dir (this is just a tech debt and they are kept there in order to have deterministic address). Also note that there were some inconsistencies in the naming of the original protocol contracts - namely some of the crates where not suffixed with "_contract" (while everywhere else it's the case). I used this PR as an opportunity to add these suffixes.
Protocol circuits are about to undergo an audit and with that also the protocol contracts. Since the Aztec.nr macros are in flux and very complicated it has been decided that we want to strip them from the protocol contracts. This has an undesirable consequence and that is that if we strip the macros we no longer get the nice Noir interface generated (e.g. `MyContract::at(address)`). To work around this problem I've created copies of the original contracts in `noir-projects/noir-contracts/contracts/protocol_interface` that keep all the function signatures but none of the implementations. With this we can use the interfaces in other Noir contracts just by updating the dependency to point to the `protocol_interface` dir instead of `protocol` dir. Note that I didn't create a copy for the `ContractClassRegistry` as that one was never used in any other contract (and I don't think there is an expectation of this ever being the case). This was also not done for the `MultiCallEntrypoint` and `PublicChecks` contracts as those are not protocol contracts even though they are in the `protocol/` dir (this is just a tech debt and they are kept there in order to have deterministic address). Also note that there were some inconsistencies in the naming of the original protocol contracts - namely some of the crates where not suffixed with "_contract" (while everywhere else it's the case). I used this PR as an opportunity to add these suffixes.
These tests had been added during one of the first iterations on mbps but never worked properly. They have been superseded by the epochs_mbps e2e test recently added.
These tests had been added during one of the first iterations on mbps but never worked properly. They have been superseded by the epochs_mbps e2e test recently added.
This will make them render more nicely in the docsite.
BEGIN_COMMIT_OVERRIDE chore: Update test chonk vk script (#19932) END_COMMIT_OVERRIDE
This PR aims to help reduce the flakiness caused by prometheus port forwarding and k8 provisioning during tests. - explicitly set timeout to be less than 6 hours so that workflow can complete network teardown - when rolling pods, wait for every state to progress before continuing, wait for PVC to be completely wiped, and roll HA db first and initialize the DB, this will reduce k8 provisioning flakes - reestablish prometheus port forwards on failure for prover-node test and wait for proven chain to update before killing prover broker to make the test more consistent - add component label to HA postgres DB
Discussed in DMs with @mverzilli that it doesn't make sense to pass around anchor block store to the execution oracles given that the anchor block needs to be fixed during the duration of the execution. Hence in this PR I am dropping it from the relevant constructors and instead just an anchor block header is passed around. Related to this also makes sense to make the `AnchorBlockStore` work solely in memory and not with persistent db as we always first sync the block header from node before simulation. This will be tackled in a followup PR.
Discussed in DMs with @mverzilli that it doesn't make sense to pass around anchor block store to the execution oracles given that the anchor block needs to be fixed during the duration of the execution. Hence in this PR I am dropping it from the relevant constructors and instead just an anchor block header is passed around. Related to this also makes sense to make the `AnchorBlockStore` work solely in memory and not with persistent db as we always first sync the block header from node before simulation. This will be tackled in a followup PR.
…flow ## Summary - Fixes the nightly-docs-release workflow which has been failing since Jan 27 - Fixes the same issue in release-please workflow (preventive) ## Problem PR #19284 added a "Build aztec CLI" step that runs `yarn-project/bootstrap.sh`, but this fails because yarn-project has portal dependencies on `noir/packages/` (e.g., `@aztec/noir-acvm_js`). This directory is only created by `noir/bootstrap.sh` and is not tracked in git. **Error:** ``` Error: @aztec/noir-acvm_js@portal:../noir/packages/acvm_js: Manifest not found ``` ## Solution Add a step to initialize the noir submodule and build the JS packages before running yarn-project bootstrap in both workflows. ## Test plan - [ ] Trigger the nightly docs workflow manually and verify it succeeds 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Josh Crites <[email protected]>
…flow (#20053) ## Summary - Fixes the nightly-docs-release workflow which has been failing since Jan 27 - Fixes the same issue in release-please workflow (preventive) ## Problem PR #19284 added a "Build aztec CLI" step that runs `yarn-project/bootstrap.sh`, but this fails because yarn-project has portal dependencies on `noir/packages/` (e.g., `@aztec/noir-acvm_js`). This directory is only created by `noir/bootstrap.sh` and is not tracked in git. **Error:** ``` Error: @aztec/noir-acvm_js@portal:../noir/packages/acvm_js: Manifest not found ``` ## Solution Add a step to initialize the noir submodule and build the JS packages before running yarn-project bootstrap in both workflows. ## Test plan - [ ] Trigger the nightly docs workflow manually and verify it succeeds 🤖 Generated with [Claude Code](https://claude.ai/code)
We had added sleeps to the slasher stop method to workaround [this viem bug](wevm/viem#3714), but it was fixed in [version 2.31.2](wevm/viem@ac4f036), and we're using `2.38.2`. This removes those sleeps so tests run fast on CI.
Bug caught by our e2e tests! http://ci.aztec-labs.com/d7587ba63f1749e0 This should fix it :) The proposer didn't have its own proposal in the attestation pool. This is an issue because, once the proposal is sent, it becomes a so-called `pinned_peer` for the peers that have received it. The pinned peer implementation logic is that we always expect it to have a proposal (since that defines a peer as "pinned peer"). The proposer not having its own proposal in the attestation pool undermines this assumption.
Should fix [this flake](http://ci.aztec-labs.com/1b4aac6e01f9b027). The test waits until `getPendingOffenses` in the store is not empty, but then the `getProposerActions` depends on `getOffensesForRound`, which gets updated _after_ `getPendingOffenses`. We should be using txs for making all db updated atomic.
The following methods have been removed from the `AztecNode` interface: - `getNullifierSiblingPath` - `getNoteHashSiblingPath` - `getArchiveSiblingPath` - `getPublicDataSiblingPath` These methods were not used by PXE and returned a subset of the information already available through the corresponding membership witness methods. Hence doesn't make sense to keep them around. This has been agreed upon with the alpha team [here](https://aztecprotocol.slack.com/archives/C04BTJAA694/p1769624408890879).
We had added sleeps to the slasher stop method to workaround [this viem bug](wevm/viem#3714), but it was fixed in [version 2.31.2](wevm/viem@ac4f036), and we're using `2.38.2`. This removes those sleeps so tests run fast on CI.
This PR includes some refactors to our comptime code so it's interpreted faster: 1. a `CHashMap` type is introduced. This is a dummy HashMap that does lookups by traversing an entire list. At first this might seem slower. However, `UHashMap` does a lot of things to get or insert a key (it hashes is, computing the hash using poseidon2, then does a linear probing, etc.), and most of that time is then spent in the interpreter and not actually in the logic. The linear logic ends up being much faster, even for big maps (and here the maps are small, but it still makes a difference). See also [this PR](noir-lang/noir#11384). 2. Use `CtString` more to build function signatures and so on. This is a bit more efficient than working with `Quoted` 3. Eventually I removed `AsStrQuote`. There are two places where we need the length of a `CtString` but then I inlined the code (it's a one-line thing). For `token_contract`, the time to re-check main in LSP goes from 270ms to 220ms. 50ms is not much, but from an IDE-perspective it is. Then `nargo check` for `token_contract` goes from 1.13 seconds to 1.07 seconds. Again, not a huge win, but it's noticeable in the IDE. (this should improve the LSP speed of all contracts, not just token_contract, but that's one of the biggest ones so I always use it to try things on)
This PR includes some refactors to our comptime code so it's interpreted faster: 1. a `CHashMap` type is introduced. This is a dummy HashMap that does lookups by traversing an entire list. At first this might seem slower. However, `UHashMap` does a lot of things to get or insert a key (it hashes is, computing the hash using poseidon2, then does a linear probing, etc.), and most of that time is then spent in the interpreter and not actually in the logic. The linear logic ends up being much faster, even for big maps (and here the maps are small, but it still makes a difference). See also [this PR](noir-lang/noir#11384). 2. Use `CtString` more to build function signatures and so on. This is a bit more efficient than working with `Quoted` 3. Eventually I removed `AsStrQuote`. There are two places where we need the length of a `CtString` but then I inlined the code (it's a one-line thing). For `token_contract`, the time to re-check main in LSP goes from 270ms to 220ms. 50ms is not much, but from an IDE-perspective it is. Then `nargo check` for `token_contract` goes from 1.13 seconds to 1.07 seconds. Again, not a huge win, but it's noticeable in the IDE. (this should improve the LSP speed of all contracts, not just token_contract, but that's one of the biggest ones so I always use it to try things on)
When the updates to set out hash root in checkpoint was merged after the escape hatch, it inserted a subtle bug. When the escape hatch is open, the check was skipped because of an early return. This meant that during the escape hatch, it was possible for someone to propose completely sound blocks, but for someone else to then prove it with a different outhash if they found soundness bugs 😱. When the updates where made in `EscapeHAtchIntegrationBase` it simple inserted `bytes32(0)` as the `outhash` meaning that the actual test was doing exactly this (since they are run without a real verifier).
Initial implementation of the capabilities manifest for app<->wallet communication. Allows a dapp to request permissions/access upfront to the wallet, which makes UX so much better. It also doubles as a way for wallets to convey they support extensions to their interface. This is not intended to make communication stateful, and it is always optional for an app to request. They can just use the interface as they please and allow the wallet to prompt the user for permissions as they see fit.
Initial implementation of the capabilities manifest for app<->wallet communication. Allows a dapp to request permissions/access upfront to the wallet, which makes UX so much better. It also doubles as a way for wallets to convey they support extensions to their interface. This is not intended to make communication stateful, and it is always optional for an app to request. They can just use the interface as they please and allow the wallet to prompt the user for permissions as they see fit.
mralj
requested review from
IlyasRidhuan,
LHerskind,
LeilaWang,
Maddiaa0,
charlielye,
dbanks12,
fcarreiro,
jeanmon and
just-mitch
as code owners
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.