feat: staking asset handler by just-mitch · Pull Request #12968 · AztecProtocol/aztec-packages

just-mitch · 2025-03-23T19:44:50Z

just-mitch · 2025-03-23T19:45:05Z

feat: staking asset handler #12968 👈 (View in Graphite)
master

This stack of pull requests is managed by Graphite. Learn more about stacking.

LHerskind

Think there is a bunch of assume that should be bound and some of the tests don't check that the state is as expected but just the events.

l1-contracts/src/mock/StakingAssetHandler.sol

LHerskind · 2025-03-25T10:49:27Z

l1-contracts/src/mock/StakingAssetHandler.sol

+    require(!needsToMint || canMint, NotEnoughTimeSinceLastMint(lastMintTimestamp, minMintInterval));
+    if (needsToMint) {
+      require(maxDepositsPerMint > 0, CannotMintZeroAmount());
+      require(


This checks seems to weird to me.

This could still mean that you have a case that will let you overflow very easily. Not sure what the check is really for.

What are you actually protecting against here? Also the check is completely independent of the addValidator as none ofthose values are changed here, so if the check is to exists, it don't make sense here, you are practically checking if constant thing is true from this functions point of view.

My thinking was to give some guidance if the user sets bad values for their configuration (probably by accident), without needing to run through the checks whenever the things change. So at least with this check, the flow would be:

user sets bad value for, e.g. MaxDepositsPerMint

goes to add validator

explodes with this error, rather than overflow

I will remove the check though, since if we're going to add configuration validity checks we ought to do them in the setters.

I think it is fine with the guidence, but it is the wrong place here because it is not when you actually set the value. Also, overflows will throw an error, so it is just a different error 🤷.

l1-contracts/src/mock/StakingAssetHandler.sol

l1-contracts/test/staking_asset_handler/setMaxDepositsPerMint.t.sol

l1-contracts/test/staking_asset_handler/setWithdrawer.t.sol

l1-contracts/test/staking_asset_handler/setMinMintInterval.t.sol

LHerskind

Having the check when we update instead of when it is used would be good.

Fine for @Maddiaa0 to give input if he got time as well.

l1-contracts/src/mock/StakingAssetHandler.sol

LHerskind · 2025-03-25T20:49:00Z

l1-contracts/src/mock/StakingAssetHandler.sol

+    emit IntervalUpdated(_interval);
+  }
+
+  function setDepositsPerMint(uint256 _depositsPerMint) external override onlyOwner {


Do sanity check here when it is updated instead. That way we only pay for it when there is a real probability to invalidate it.

LHerskind · 2025-03-25T21:00:32Z

l1-contracts/test/staking_asset_handler/addValidator.t.sol

+    vm.assume(_attester != address(0));
+    vm.assume(_proposer != address(0));
+
+    vm.expectRevert(abi.encodeWithSelector(IStakingAssetHandler.CannotMintZeroAmount.selector));


As mentioned think this one should probably be checked when the value is set, not when it is used.

LHerskind · 2025-03-25T21:01:10Z

l1-contracts/test/staking_asset_handler/addValidator.t.sol

+
+    _depositsPerMint = bound(_depositsPerMint, 1, 1e18);
+
+    deal(address(stakingAsset), address(stakingAssetHandler), MINIMUM_STAKE * _depositsPerMint);


just-mitch · 2025-03-25T22:51:48Z

As noted, I'd like to keep the option open to set the mints per deposit to zero to be able to pause the contract.

LHerskind · 2025-03-26T08:32:43Z

As noted, I'd like to keep the option open to set the mints per deposit to zero to be able to pause the contract.

You can already do that by removing it as a minter though? And if you want to have it pause, seems simpler to add a pause in that case 🤷

just-mitch · 2025-03-26T14:08:09Z

That's a great point! Okay I will remove. No point in keeping it 👍

Fix #12932

🤖 I have created a new Aztec Packages release --- ## [0.82.3](v0.82.2...v0.82.3) (2025-03-27) ### Features * `msgpack` encoding for `Program` and `WitnessStack` ([#12841](#12841)) ([1e58eb1](1e58eb1)) * 64 bit log type id, 64 bit log metadata ([#12956](#12956)) ([20d734a](20d734a)) * AVM parsing tag validation ([#12936](#12936)) ([56b1f0d](56b1f0d)) * **avm:** add calldata & returndata to context ([#13008](#13008)) ([f03b2e5](f03b2e5)) * **avm:** merkle db hints (part 1) ([#12922](#12922)) ([34ec9e8](34ec9e8)) * **avm:** merkle hints (part 2) ([#13077](#13077)) ([fbbc6c7](fbbc6c7)) * **avm:** vm2 initial context ([#12972](#12972)) ([e2b1361](e2b1361)) * benchmark avm simulator ([#12985](#12985)) ([00fae1b](00fae1b)) * client flows benchmarks ([#13007](#13007)) ([9bf7568](9bf7568)) * gas benchmark for "normal usage" ([#13073](#13073)) ([4eb1156](4eb1156)) * Implement merkle writes in the merkle check gadget ([#13050](#13050)) ([c94fe50](c94fe50)) * LogEncryption trait ([#12942](#12942)) ([0b7e564](0b7e564)) * Node snapshot sync ([#12927](#12927)) ([afde851](afde851)), closes [#12926](#12926) * **p2p:** capture all gossipsub metrics ([#12930](#12930)) ([cc940cb](cc940cb)) * Prover node snapshot sync ([#13097](#13097)) ([1e77efb](1e77efb)) * staking asset handler ([#12968](#12968)) ([af48184](af48184)), closes [#12932](#12932) * stream crs data to disk ([#12996](#12996)) ([d016e4d](d016e4d)), closes [#12948](#12948) * track failed tests. add flake. ([f4936d7](f4936d7)) * Track test history. ([#13037](#13037)) ([036bb32](036bb32)) * track total tx fee ([#12601](#12601)) ([9612a4e](9612a4e)) * Validators sentinel ([#12818](#12818)) ([770695c](770695c)) ### Bug Fixes * added #[derive(Eq)] to EcdsaPublicKeyNote ([#12966](#12966)) ([0c21c74](0c21c74)) * Allow use of local blob sink client ([#13025](#13025)) ([ba8d654](ba8d654)) * **avm:** semicolons are hard ([#12999](#12999)) ([8871c83](8871c83)) * bootstrap network and sponsored fpc devnet ([#13044](#13044)) ([8a47d8b](8a47d8b)) * Bump tsc target ([#13052](#13052)) ([985e83b](985e83b)) * cycle_group fuzzer ([#12921](#12921)) ([69f426e](69f426e)) * **docs:** Fix import errors in aztec.js tutorial ([#12969](#12969)) ([856208a](856208a)) * **docs:** Load token artifact from the compiled source in the sample dapp tutorial ([#12802](#12802)) ([0838084](0838084)), closes [#12810](#12810) * **docs:** Update sponsored fpc docs to use 82.2 syntax ([#13054](#13054)) ([e5d425b](e5d425b)) * **e2e:** p2p ([#13002](#13002)) ([1ece539](1ece539)) * extend e2e 2 pxes timeout. strip color codes for error_regex. ([73820e4](73820e4)) * flake ([6cc9e81](6cc9e81)) * fuzzer on staking asset handler constructor test ([#13101](#13101)) ([d936285](d936285)) * invalid getCommittee function ([#13072](#13072)) ([327341f](327341f)) * mac publish should use clang 18 like x-compiler, and use it ([#12983](#12983)) ([7b83c45](7b83c45)) * make circuit parsing deterministic ([#11772](#11772)) ([76ef873](76ef873)) * parse away trailing slash from consensus host ([#12577](#12577)) ([6701806](6701806)) * prerelease versions should be pushed to install.aztec.network ([#13086](#13086)) ([c4e6039](c4e6039)) * smoke ([#13060](#13060)) ([7756b15](7756b15)) * some flake additions ([58638f1](58638f1)) * sponsored fpc arg parsed correctly ([#12976](#12976)) ([#12977](#12977)) ([a85f530](a85f530)) * starting the sandbox with no pxe should still deploy initial test accounts ([#13047](#13047)) ([d92d895](d92d895)) * Syntax error when running tests via jest after tsc build ([#13051](#13051)) ([f972db9](f972db9)) * Use the correct image in aztec start ([#13058](#13058)) ([06285cd](06285cd)) * yolo fix ([91e2f4b](91e2f4b)) * yolo fix nightly ([b3b3259](b3b3259)) * yolo fix obvious thing to track fails. ([2fee630](2fee630)) * yolo flakes ([e3b030a](e3b030a)) * yolo set -x ([bfd3205](bfd3205)) * yolo we suspect the halt is making tests fail that would have passed ([04e3fa2](04e3fa2)) ### Miscellaneous * `getIndexedTaggingSecretAsSender` oracle cleanup ([#13015](#13015)) ([8e71e55](8e71e55)) * Add a script to generate cpp files for AVM2 ([#13091](#13091)) ([7bb43a9](7bb43a9)) * add default native proving for cli wallet ([#12855](#12855)) ([c0f773c](c0f773c)) * add default native proving for cli wallet retry ([#13028](#13028)) ([b2f4785](b2f4785)) * Alpha testnet into master ([#13033](#13033)) ([d98fdbd](d98fdbd)) * AVM TS - move tag validation outside of instruction constructors ([#13038](#13038)) ([45548ab](45548ab)), closes [#12934](#12934) * **avm:** final codegen nuking ([#13089](#13089)) ([9c82f3f](9c82f3f)) * **avm:** remove codegen (all but flavor) ([#13079](#13079)) ([e1f2bdd](e1f2bdd)) * **bb:** minor acir buf C++ improvements ([#13042](#13042)) ([1ebd044](1ebd044)) * boxes dep cleanup ([#12979](#12979)) ([6540b7c](6540b7c)) * **ci:** less catch all e2e_p2p flakes ([#12737](#12737)) ([2134634](2134634)) * comprehensive cleanup of translator flavor and use inheritance properly in flavors ([#13041](#13041)) ([dc5f78f](dc5f78f)) * compress storage footprint ([#12871](#12871)) ([58c110f](58c110f)) * display warning when installing bb versions < 0.82.0 ([#13027](#13027)) ([7247fe7](7247fe7)) * **docs:** Update docs on fees and various other updates ([#12929](#12929)) ([1dec907](1dec907)) * dump dmesg/net/cpu/mem usage at end of ci run ([#12967](#12967)) ([8877792](8877792)) * fix governance util issue ([#13043](#13043)) ([d768d26](d768d26)) * redundant if in affine from projective constructor ([#13045](#13045)) ([3a7ba2d](3a7ba2d)) * remove addition of dummy ops in mock circuit producer ([#13003](#13003)) ([a64d1dc](a64d1dc)) * remove dummy ops in decider pk ([#13049](#13049)) ([da6d021](da6d021)) * replace relative paths to noir-protocol-circuits ([e1b88f6](e1b88f6)) * replace relative paths to noir-protocol-circuits ([849b4b0](849b4b0)) * replace relative paths to noir-protocol-circuits ([18a02d6](18a02d6)) * Revert "chore: add default native proving for cli wallet ([#12855](#12855))" ([#13013](#13013)) ([98e2576](98e2576)) * Speed up and deflake sentinel test ([#13078](#13078)) ([27f1eca](27f1eca)) * **testnet:** making consensus host mandatory input ([#12716](#12716)) ([d47c74a](d47c74a)) * towards no more mock op_queues ([#12984](#12984)) ([fefffa7](fefffa7)) * update bb version for noir 1.0.0-beta.0+ ([#13026](#13026)) ([dd68074](dd68074)) * update CODEOWNERS to reflect new sync method ([#12998](#12998)) ([a3d1915](a3d1915)) ### Documentation * Add fees to cli reference ([#12884](#12884)) ([4a0fd58](4a0fd58)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

just-mitch linked an issue Mar 23, 2025 that may be closed by this pull request

feat: staking asset handler #12932

Closed

just-mitch force-pushed the 12932-feat-staking-asset-handler branch 4 times, most recently from cec4505 to fa5e27e Compare March 24, 2025 19:02

just-mitch marked this pull request as ready for review March 24, 2025 19:03

just-mitch changed the title ~~staking asset handler~~ feat: staking asset handler Mar 24, 2025

just-mitch force-pushed the 12932-feat-staking-asset-handler branch from fa5e27e to 7bea5cb Compare March 24, 2025 19:11

LHerskind self-requested a review March 25, 2025 10:29

LHerskind reviewed Mar 25, 2025

View reviewed changes

just-mitch force-pushed the 12932-feat-staking-asset-handler branch 2 times, most recently from 69f0538 to 2a3540c Compare March 25, 2025 16:31

LHerskind approved these changes Mar 25, 2025

View reviewed changes

staking asset handler

291cbe3

just-mitch force-pushed the 12932-feat-staking-asset-handler branch from 2a3540c to 291cbe3 Compare March 26, 2025 14:19

just-mitch merged commit af48184 into master Mar 26, 2025
8 checks passed

just-mitch deleted the 12932-feat-staking-asset-handler branch March 26, 2025 15:38

AztecBot mentioned this pull request Mar 26, 2025

chore(master): release 0.82.3 #12980

Merged

DanielKotov pushed a commit that referenced this pull request Mar 27, 2025

feat: staking asset handler (#12968)

eae60cc

Fix #12932

DanielKotov pushed a commit that referenced this pull request Mar 27, 2025

feat: staking asset handler (#12968)

2c64946

Fix #12932


		_depositsPerMint = bound(_depositsPerMint, 1, 1e18);

		deal(address(stakingAsset), address(stakingAssetHandler), MINIMUM_STAKE * _depositsPerMint);

Conversation

just-mitch commented Mar 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

just-mitch commented Mar 23, 2025

Uh oh!

LHerskind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LHerskind Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

just-mitch Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

just-mitch Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

LHerskind Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

LHerskind left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

LHerskind Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

LHerskind Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

LHerskind Mar 25, 2025

Choose a reason for hiding this comment

Uh oh!

just-mitch commented Mar 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

LHerskind commented Mar 26, 2025

Uh oh!

just-mitch commented Mar 26, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

just-mitch commented Mar 23, 2025 •

edited

Loading

just-mitch commented Mar 25, 2025 •

edited

Loading