fix: enclave contracts audit remediation [skip-line-limit]#1545
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImplements pull-payment rewards/treasury accounting, timestamp-based requestBlock and token clocks, stricter slashing with lanes and two-step bans, verifier domain/VK binding, exit-queue fixes, registry/token hardening, ABI/interface/artifact updates, deployment scripts/config changes, and comprehensive docs/tests alignment. ChangesCore protocol refactor and audit hardening
Sequence Diagram(s)(skipped) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/enclave-contracts/tasks/ciphernode.ts (1)
264-270:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't permanently disable ENCL restrictions from a mint helper.
disableTransferRestrictions()is a one-way, global admin action. Callingciphernode:mint-tokensfor one test wallet now permanently changes transfer policy for the whole deployment, which is a much bigger blast radius than this task advertises.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/enclave-contracts/tasks/ciphernode.ts` around lines 264 - 270, The task currently calls enclaveTokenContract.disableTransferRestrictions() when transfersRestricted is true (in the ciphernode:mint-tokens flow), which is a permanent, global admin change; remove or stop invoking disableTransferRestrictions() from the mint helper and instead perform minting without altering global transfer policy—either mint directly to the target address (using the contract's mint/ownerMint function) or, if transfers must be enabled for this single wallet, implement a scoped approach (require an explicit admin-only flag on the task or use a temporary allowance/whitelist function if the contract exposes one) and never call disableTransferRestrictions() from ciphernode:mint-tokens. Ensure references to transfersRestricted and enclaveTokenContract.disableTransferRestrictions() are the spots you update.packages/enclave-contracts/tasks/enclave.ts (1)
226-239:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPopulate
maxFeefrom the quote before callingrequest().This task now hardcodes
maxFee: 0n, but then immediately quotes a fee and approves it. With max-fee enforcement enabled, any nonzero quote will makerequest()revert.💡 Suggested fix
- const requestParams = { + const requestParamsBase = { committeeSize, inputWindow: [inputWindowStart, inputWindowEnd] as [ BigNumberish, BigNumberish, ], e3Program: e3Address === ZeroAddress ? mockE3ProgramArgs!.address : e3Address, paramSet, computeProviderParams, customParams, proofAggregationEnabled, - maxFee: 0n, }; - console.log("Request parameters:", requestParams); - - const fee = await enclaveContract.getE3Quote(requestParams); + const fee = await enclaveContract.getE3Quote({ + ...requestParamsBase, + maxFee: 0n, + }); + const requestParams = { + ...requestParamsBase, + maxFee: fee, + }; + + console.log("Request parameters:", requestParams);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/enclave-contracts/tasks/enclave.ts` around lines 226 - 239, The requestParams currently hardcodes maxFee: 0n which will cause request() to revert when a nonzero quote is obtained; instead, after you call whatever function produces the fee quote (the variable holding the quote/fee in this scope — e.g., quote, quoteResult, or quotedFee), set requestParams.maxFee to that quoted value (converted to the expected BigNumberish/BigInt) before calling request(); ensure the same quoted value is used for the approval step so requestParams.maxFee matches the approved fee and that request() will not revert.packages/enclave-contracts/contracts/registry/CiphernodeRegistryOwnable.sol (1)
248-249:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEmit the finalized committee, not caller-supplied
nodes.The proof is verified against
c.topNodes, butCommitteePublishedstill echoesnodesfrom calldata. A caller can publish with any same-length array and make the event lie about which committee was finalized, which is enough to break indexers or off-chain consumers that trust the event payload. Either enforce element-by-element equality or emitc.topNodesdirectly.Also applies to: 254-265, 273-273
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/enclave-contracts/contracts/registry/CiphernodeRegistryOwnable.sol` around lines 248 - 249, The CommitteePublished event currently emits the caller-supplied `nodes` array which may differ from the verified committee stored in `c.topNodes`; change the implementation to emit `c.topNodes` (the finalized committee verified by the proof) instead of `nodes`, or add an explicit element-by-element equality check between `nodes` and `c.topNodes` before emitting to guarantee the event matches the on-chain committee; update all occurrences around the `CommitteePublished` emission (including the blocks referencing `nodes` at lines ~248, 254–265, 273) to reference `c.topNodes` or enforce exact equality.
🧹 Nitpick comments (1)
packages/enclave-contracts/contracts/interfaces/IBondingRegistry.sol (1)
282-290: ⚡ Quick winRename the EIP-6372 argument away from
blockNumber.The NatSpec now correctly describes a timestamp timepoint, but the parameter name still suggests callers should pass a block height. Since parameter names are ABI-irrelevant, this is a cheap clarity win.
Possible cleanup
/** * `@notice` Get operator's ticket balance at a specific timepoint (EIP-6372). * `@dev` The ticket token uses {block.timestamp} for its voting clock. * `@param` operator Address of the operator - * `@param` blockNumber Timepoint (block.timestamp) to query + * `@param` timepoint Timepoint (block.timestamp) to query * `@return` Ticket balance at the specified timepoint */ function getTicketBalanceAtBlock( address operator, - uint256 blockNumber + uint256 timepoint ) external view returns (uint256);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/enclave-contracts/contracts/interfaces/IBondingRegistry.sol` around lines 282 - 290, Rename the misleading parameter name in the getTicketBalanceAtBlock function signature (and any matching declarations/implementations) from blockNumber to a clearer name like timepoint or timestamp; update the function signature in the IBondingRegistry interface (function getTicketBalanceAtBlock(address operator, uint256 timepoint)) and adjust any internal references and the NatSpec `@param` tag to use the new name so callers/readers understand it expects a timestamp (EIP-6372 timepoint) rather than a block height.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/flow-trace/05_FAILURE_REFUND_SLASHING.md`:
- Around line 781-784: Update the documentation in the flow-trace so it matches
the contract logic in _applySlashedFunds: when dist.honestNodeCount == 0 and
toHonestNodes > 0, the implementation routes toHonestNodes into the treasury
pull credits (not toRequester); change the lines referencing redirect
toRequester to instead state that funds are routed to the treasury pull credits,
and mention the exact symbols _applySlashedFunds, honestNodeCount,
toHonestNodes, and toRequester so readers can correlate the doc to the contract
behavior.
In
`@packages/enclave-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.json`:
- Around line 435-440: The getSlashProposal() read path does not include the
Lane value even though SlashProposed and SlashExecuted emit lane; update the
proposal API so callers can read lane without replaying events by either adding
a lane (enum ISlashingManager.Lane / uint8) field to the returned SlashProposal
tuple from getSlashProposal() (and corresponding struct/type if present) or by
adding a dedicated getter like getSlashProposalLane(uint256 proposalId) that
returns ISlashingManager.Lane; update any ABI/JSON artifacts (and the function
signature for getSlashProposal or the new getter) so the returned data includes
the lane value consistently with the SlashProposed/SlashExecuted events.
In `@packages/enclave-contracts/contracts/interfaces/IE3RefundManager.sol`:
- Around line 229-232: The function
IE3RefundManager.claimSlashedFundsOnSuccessBatch(uint256[] calldata e3Ids)
currently returns a single uint256 totalClaimed which is ambiguous when batched
E3s use different paymentToken denominations; update the interface to avoid an
untyped aggregate by either (A) enforcing homogeneous tokens up-front (validate
all e3Ids map to the same paymentToken and revert if mixed) or (B) remove the
uint256 totalClaimed return and instead rely on per-claim events or return a
per-token/result structure (e.g., mapping token->amount or an array of per-e3
results) so callers get amounts typed by token; make the change in the signature
and document/emits per-claim events in the implementation accordingly.
In `@packages/enclave-contracts/contracts/interfaces/IEnclave.sol`:
- Around line 795-801: The claimRewards function currently returns an aggregate
uint256 totalClaimed which is misleading because e3Ids may use different fee
tokens; update the interface by removing the uint256 totalClaimed return and
make claimRewards(uint256[] calldata e3Ids) external (no return), then update
all implementing contracts, tests and callers to stop relying on totalClaimed
and instead use per-E3 events or explicit per-E3 return types (e.g., an array of
structs {e3Id, token, amount}) if you need per-token totals; reference the
function name claimRewards and the return symbol totalClaimed when making these
changes so implementations (and the ABI) are kept consistent.
In `@packages/enclave-contracts/contracts/registry/BondingRegistry.sol`:
- Around line 82-84: The new storage slot authorizedDistributorCount was added
before existing state (slashedFundsTreasury, ticketPrice, exitDelay, etc.),
which corrupts storage for upgradeable contracts; remove
authorizedDistributorCount from this location or move it to the very end of the
contract's storage layout and add an explicit migration/reinitializer that reads
the existing authorizedDistributors mapping/array and initializes
authorizedDistributorCount for existing deployments; ensure the migration
function is a properly versioned initializer (e.g., reinitializer) and reference
authorizedDistributors, authorizedDistributorCount, slashedFundsTreasury,
ticketPrice, and exitDelay when implementing the fix.
In `@packages/enclave-contracts/contracts/slashing/SlashingManager.sol`:
- Around line 775-791: Both unban paths leave a stale pending ban that allows a
later confirmBan to re-ban the node; in unbanNode and in updateBanStatus (when
status is false) clear the pending proposal by deleting or zeroing
_pendingBans[node] (the same storage used by proposeBan/confirmBan/cancelBan)
before setting banned[node] = false and emitting NodeBanUpdated so a prior
proposal cannot be confirmed later.
In `@packages/enclave-contracts/contracts/test/MockBlacklistUSDC.sol`:
- Around line 37-44: The _update override currently rejects transfers if either
party is blacklisted, which incorrectly blocks mint/burn since those use
address(0) as a sentinel; modify _update (the function named _update) to skip
blacklist checks for the zero address by only reverting on isBlacklisted[from]
when from != address(0) and only reverting on isBlacklisted[to] when to !=
address(0), then call super._update(from, to, value).
In `@packages/enclave-contracts/contracts/test/MockFeeOnTransferToken.sol`:
- Around line 10-13: The contract docs state fees are "burned" but the transfer
logic sends the fee to address(0xdead), which does not reduce totalSupply;
update the implementation so true burns occur by sending fees to address(0)
(replace address(0xdead) usages in the transfer/transferFrom fee handling) or
alternatively change the NatSpec/comments to say fees are redirected to a sink
address; locate the fee handling in MockFeeOnTransferToken (the
transfer/transferFrom fee calculation and the address(0xdead) recipient) and
make the change consistently so totalSupply semantics match the documentation.
In `@packages/enclave-contracts/contracts/token/EnclaveToken.sol`:
- Around line 112-123: The constructor grants DEFAULT_ADMIN_ROLE, MINTER_ROLE
and WHITELIST_ROLE to initialOwner_ but ownership transfers via Ownable2Step
(transferOwnership / acceptOwnership) do not move those roles, leaving the old
owner still role-admin; fix by synchronizing roles during ownership handover:
override transferOwnership and acceptOwnership (or hook into the Ownable2Step
flow) to _grantRole(DEFAULT_ADMIN_ROLE, newOwner), _grantRole(MINTER_ROLE,
newOwner), _grantRole(WHITELIST_ROLE, newOwner) and _revokeRole(...) for the
previous owner atomically after acceptOwnership completes, or alternatively
remove onlyOwner usages and keep role-only access; ensure renounceOwnership()
behavior is considered (if disabled, keep roles in sync) and use the same role
symbols (_grantRole, _revokeRole, DEFAULT_ADMIN_ROLE, MINTER_ROLE,
WHITELIST_ROLE, owner(), transferOwnership, acceptOwnership) so reviewers can
locate the changes.
In `@packages/enclave-contracts/ignition/modules/bfvDecryptionVerifier.ts`:
- Around line 21-28: The parameters c6FoldKeyHash and c7KeyHash are currently
read via m.getParameter with an implicit bytes32(0) default, allowing
deployments to proceed with placeholder VK hashes; change the handling in the
code that reads these parameters (the calls to m.getParameter for
"c6FoldKeyHash" and "c7KeyHash") to validate the returned value and reject or
throw if it equals the zero hash (0x...00) instead of silently accepting the
default—ensure the deployment fails fast with a clear error when either
c6FoldKeyHash or c7KeyHash equals bytes32(0), and update any related messages to
reference the specific parameter name for clarity.
In `@packages/enclave-contracts/ignition/modules/bfvPkVerifier.ts`:
- Around line 19-26: The parameters nodesFoldKeyHash and c5KeyHash must not
silently default to bytes32(0); update the m.getParameter calls in
bfvPkVerifier.ts to require explicit values (remove the zero-value default or
fetch without fallback) and add a runtime validation that checks
nodesFoldKeyHash and c5KeyHash are not the all-zero bytes32 value (e.g., compare
to "0x000...000") and throw a clear error/abort if they are; reference the
symbols nodesFoldKeyHash, c5KeyHash and the module/method using m.getParameter
to locate and implement this validation.
In `@packages/enclave-contracts/ignition/modules/slashingManager.ts`:
- Around line 13-15: Validate the admin delay returned by m.getParameter before
constructing SlashingManager: after const initialDelay =
m.getParameter("initialDelay", DEFAULT_ADMIN_DELAY); check if initialDelay is <=
0 and throw a clear error (e.g. "initialDelay must be a positive non-zero
value") or fallback to DEFAULT_ADMIN_DELAY, then pass the validated value into
m.contract("SlashingManager", [initialDelay, admin]). This change ensures
initialDelay (used for SlashingManager) cannot be zero or negative.
In `@packages/enclave-contracts/scripts/deployAndSave/slashingManager.ts`:
- Around line 45-47: The computed delay value (const delay = initialDelay !==
undefined ? BigInt(initialDelay) : DEFAULT_ADMIN_DELAY) must be validated so
zero or negative delays are rejected; modify the deployment code that sets delay
to check the normalized BigInt value (delay) and throw or exit with an error if
delay <= 0n, referencing the symbols initialDelay, delay, and
DEFAULT_ADMIN_DELAY so the validation occurs immediately after normalization and
before any use of delay in admin handover or contract deployment.
In `@packages/enclave-contracts/scripts/deployEnclave.ts`:
- Around line 285-296: The check for SLASHER_ROLE happens before the grant
transaction is mined; change the flow in the slashingManager usage so
addSlasher(...) is awaited as a transaction receipt before calling hasRole: call
slashingManager.addSlasher(slasherAddress), store the returned tx, await
tx.wait(), then read slashingManager.SLASHER_ROLE() and call
hasRole(slasherRole, slasherAddress) to verify; reference symbols: addSlasher,
SLASHER_ROLE, hasRole, slashingManager.
In `@packages/enclave-contracts/scripts/validateUpgrade.ts`:
- Around line 42-55: The UPGRADEABLE_CONTRACTS array uses incorrect source paths
("project/contracts/...") so loadLayout() cannot find entries in
buildInfo.output.contracts[source][contract]; update each source string in
UPGRADEABLE_CONTRACTS to the actual Hardhat source keys used in this PR (e.g.,
"contracts/Enclave.sol", "contracts/registry/CiphernodeRegistryOwnable.sol",
etc.) so that loadLayout() and the lookup
buildInfo.output.contracts[source][contract] resolve correctly for functions
that validate upgrades.
In `@packages/enclave-contracts/test/fixtures/constants.ts`:
- Around line 23-25: The ENCRYPTION_SCHEME_ID constant is hardcoded to
keccak256("fhe.bfv.2.0") but verifier wrappers expect keccak256("fhe.rs:BFV");
update the fixture so the constant is derived from the canonical string to avoid
drift—replace the hardcoded hex with a value computed from
keccak256("fhe.rs:BFV") (or compute at runtime using
utils.keccak256(utils.toUtf8Bytes("fhe.rs:BFV"))) and export that as
ENCRYPTION_SCHEME_ID so tests and the BFV verifier wrapper use the exact same
identifier.
In `@packages/enclave-contracts/test/fixtures/system.ts`:
- Around line 186-189: The return type for the fixture incorrectly marks
operator1, operator2, and operator3 as required Signer instances even though
they can be undefined when setupOperators < 3; update the
type/interface/returned object so operator1, operator2, and operator3 are
optional (e.g., Signer | undefined or mark with ?) wherever they’re
declared/returned (references: operator1, operator2, operator3 in the
fixture/contract), and apply the same optional typing change to the other
occurrence of these fields mentioned in the review.
- Around line 225-227: The test fixture can push undefined into operators when
setupOperators is larger than the available signers; add a validation before the
loop that ensures setupOperators is <= signers.length - 2 (because you start at
index 2), and if it isn't, throw a clear error (or clamp setupOperators) so the
for loop that pushes signers[2 + i] into operators never inserts undefined;
reference the variables setupOperators, operators, and signers and perform the
check prior to the for loop that populates operators.
---
Outside diff comments:
In `@packages/enclave-contracts/contracts/registry/CiphernodeRegistryOwnable.sol`:
- Around line 248-249: The CommitteePublished event currently emits the
caller-supplied `nodes` array which may differ from the verified committee
stored in `c.topNodes`; change the implementation to emit `c.topNodes` (the
finalized committee verified by the proof) instead of `nodes`, or add an
explicit element-by-element equality check between `nodes` and `c.topNodes`
before emitting to guarantee the event matches the on-chain committee; update
all occurrences around the `CommitteePublished` emission (including the blocks
referencing `nodes` at lines ~248, 254–265, 273) to reference `c.topNodes` or
enforce exact equality.
In `@packages/enclave-contracts/tasks/ciphernode.ts`:
- Around line 264-270: The task currently calls
enclaveTokenContract.disableTransferRestrictions() when transfersRestricted is
true (in the ciphernode:mint-tokens flow), which is a permanent, global admin
change; remove or stop invoking disableTransferRestrictions() from the mint
helper and instead perform minting without altering global transfer
policy—either mint directly to the target address (using the contract's
mint/ownerMint function) or, if transfers must be enabled for this single
wallet, implement a scoped approach (require an explicit admin-only flag on the
task or use a temporary allowance/whitelist function if the contract exposes
one) and never call disableTransferRestrictions() from ciphernode:mint-tokens.
Ensure references to transfersRestricted and
enclaveTokenContract.disableTransferRestrictions() are the spots you update.
In `@packages/enclave-contracts/tasks/enclave.ts`:
- Around line 226-239: The requestParams currently hardcodes maxFee: 0n which
will cause request() to revert when a nonzero quote is obtained; instead, after
you call whatever function produces the fee quote (the variable holding the
quote/fee in this scope — e.g., quote, quoteResult, or quotedFee), set
requestParams.maxFee to that quoted value (converted to the expected
BigNumberish/BigInt) before calling request(); ensure the same quoted value is
used for the approval step so requestParams.maxFee matches the approved fee and
that request() will not revert.
---
Nitpick comments:
In `@packages/enclave-contracts/contracts/interfaces/IBondingRegistry.sol`:
- Around line 282-290: Rename the misleading parameter name in the
getTicketBalanceAtBlock function signature (and any matching
declarations/implementations) from blockNumber to a clearer name like timepoint
or timestamp; update the function signature in the IBondingRegistry interface
(function getTicketBalanceAtBlock(address operator, uint256 timepoint)) and
adjust any internal references and the NatSpec `@param` tag to use the new name so
callers/readers understand it expects a timestamp (EIP-6372 timepoint) rather
than a block height.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6207f3a7-0d8b-491f-9fd2-27319ec45183
📒 Files selected for processing (71)
agent/flow-trace/02_TOKENS_AND_ACTIVATION.mdagent/flow-trace/03_E3_REQUEST_AND_COMMITTEE.mdagent/flow-trace/04_DKG_AND_COMPUTATION.mdagent/flow-trace/05_FAILURE_REFUND_SLASHING.mdagent/flow-trace/06_DEACTIVATION_AND_COMPLETION.mdpackages/enclave-contracts/artifacts/contracts/Enclave.sol/Enclave.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.jsonpackages/enclave-contracts/artifacts/contracts/registry/CiphernodeRegistryOwnable.sol/CiphernodeRegistryOwnable.jsonpackages/enclave-contracts/artifacts/contracts/token/EnclaveTicketToken.sol/EnclaveTicketToken.jsonpackages/enclave-contracts/contracts/E3RefundManager.solpackages/enclave-contracts/contracts/Enclave.solpackages/enclave-contracts/contracts/interfaces/IBondingRegistry.solpackages/enclave-contracts/contracts/interfaces/ICiphernodeRegistry.solpackages/enclave-contracts/contracts/interfaces/IDecryptionVerifier.solpackages/enclave-contracts/contracts/interfaces/IE3RefundManager.solpackages/enclave-contracts/contracts/interfaces/IEnclave.solpackages/enclave-contracts/contracts/interfaces/IPkVerifier.solpackages/enclave-contracts/contracts/interfaces/ISlashingManager.solpackages/enclave-contracts/contracts/lib/EnclavePricing.solpackages/enclave-contracts/contracts/lib/ExitQueueLib.solpackages/enclave-contracts/contracts/registry/BondingRegistry.solpackages/enclave-contracts/contracts/registry/CiphernodeRegistryOwnable.solpackages/enclave-contracts/contracts/slashing/SlashingManager.solpackages/enclave-contracts/contracts/test/MockBlacklistUSDC.solpackages/enclave-contracts/contracts/test/MockDecryptionVerifier.solpackages/enclave-contracts/contracts/test/MockFeeOnTransferToken.solpackages/enclave-contracts/contracts/test/MockPkVerifier.solpackages/enclave-contracts/contracts/token/EnclaveTicketToken.solpackages/enclave-contracts/contracts/token/EnclaveToken.solpackages/enclave-contracts/contracts/verifiers/bfv/BfvDecryptionVerifier.solpackages/enclave-contracts/contracts/verifiers/bfv/BfvPkVerifier.solpackages/enclave-contracts/hardhat.config.tspackages/enclave-contracts/ignition/modules/bfvDecryptionVerifier.tspackages/enclave-contracts/ignition/modules/bfvPkVerifier.tspackages/enclave-contracts/ignition/modules/enclave.tspackages/enclave-contracts/ignition/modules/slashingManager.tspackages/enclave-contracts/package.jsonpackages/enclave-contracts/scripts/deployAndSave/enclave.tspackages/enclave-contracts/scripts/deployAndSave/enclaveToken.tspackages/enclave-contracts/scripts/deployAndSave/slashingManager.tspackages/enclave-contracts/scripts/deployEnclave.tspackages/enclave-contracts/scripts/validateUpgrade.tspackages/enclave-contracts/tasks/ciphernode.tspackages/enclave-contracts/tasks/enclave.tspackages/enclave-contracts/test/BfvDecryptionVerifier.spec.tspackages/enclave-contracts/test/BfvPkVerifier.spec.tspackages/enclave-contracts/test/E3Lifecycle/E3Integration.spec.tspackages/enclave-contracts/test/E3Lifecycle/Sortition.spec.tspackages/enclave-contracts/test/Enclave.spec.tspackages/enclave-contracts/test/Governance/AccessAndBounds.spec.tspackages/enclave-contracts/test/Pricing/DustRotation.spec.tspackages/enclave-contracts/test/Pricing/Pricing.spec.tspackages/enclave-contracts/test/Pricing/PullPaymentsAndAllowlist.spec.tspackages/enclave-contracts/test/Registry/BondingRegistry.spec.tspackages/enclave-contracts/test/Registry/CiphernodeRegistryOwnable.spec.tspackages/enclave-contracts/test/Slashing/CommitteeExpulsion.spec.tspackages/enclave-contracts/test/Slashing/SlashingLanes.spec.tspackages/enclave-contracts/test/Slashing/SlashingManager.spec.tspackages/enclave-contracts/test/Standards/StandardsAndUpgrades.spec.tspackages/enclave-contracts/test/Token/EnclaveTicketToken.spec.tspackages/enclave-contracts/test/Token/EnclaveToken.spec.tspackages/enclave-contracts/test/fixtures/attestation.tspackages/enclave-contracts/test/fixtures/connection.tspackages/enclave-contracts/test/fixtures/constants.tspackages/enclave-contracts/test/fixtures/helpers.tspackages/enclave-contracts/test/fixtures/index.tspackages/enclave-contracts/test/fixtures/operators.tspackages/enclave-contracts/test/fixtures/system.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/enclave-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.json (1)
784-877:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExpose
laneongetSlashProposal()too.
SlashProposedandSlashExecutednow emitlane, but the proposal read path still omits it. Callers querying byproposalIdstill have to replay events to distinguish Lane A from Lane B. Either addlaneto the returned tuple or expose a dedicated getter.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/enclave-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.json` around lines 784 - 877, getSlashProposal currently returns the SlashProposal tuple without the lane field while events SlashProposed and SlashExecuted include lane, forcing callers to replay events; update the ISlashingManager interface so the SlashProposal struct (and the getSlashProposal() output tuple) includes the lane field (same type as emitted in SlashProposed/SlashExecuted) or add a dedicated getter like getSlashProposalLane(proposalId) that returns the lane; modify the returned tuple name "proposal" from getSlashProposal and the struct definition used by getSlashProposal to include the new lane member so callers can distinguish Lane A vs B directly.
🧹 Nitpick comments (1)
packages/enclave-contracts/test/BfvVkBindingIntegration.spec.ts (1)
235-247: ⚡ Quick winKeep one happy-path assertion through the wrapper
verify()entrypoints.These direct
ICircuitVerifiercalls no longer exercise the wrapper logic changed in this PR, so this suite can stay green even ifBfvPkVerifier.verify/BfvDecryptionVerifier.verifyregress on argument wiring or domain-binding checks. Please keep a positive-path wrapper test once domain-bound fixtures are available, or split this into a clearly named circuit-only test.Also applies to: 256-266
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/enclave-contracts/test/BfvVkBindingIntegration.spec.ts` around lines 235 - 247, The test currently calls the underlying ICircuitVerifier (dkgCircuit.verify) which bypasses the wrapper logic; update the spec to include at least one happy-path assertion that invokes the wrapper verify entrypoint (e.g., call BfvPkVerifier.verify and/or BfvDecryptionVerifier.verify with the decoded rawDkgProof and rawDkgPi or appropriately wrapped arguments) and expect it to return true so the wrapper wiring and domain-binding are exercised; if you want to keep the circuit-only assertion, either rename this test to indicate it is "circuit-only" or split out a separate test that calls the wrapper verify methods (BfvPkVerifier.verify / BfvDecryptionVerifier.verify) using the same decoded inputs (rawDkgProof, rawDkgPi) and assert success.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In
`@packages/enclave-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.json`:
- Around line 784-877: getSlashProposal currently returns the SlashProposal
tuple without the lane field while events SlashProposed and SlashExecuted
include lane, forcing callers to replay events; update the ISlashingManager
interface so the SlashProposal struct (and the getSlashProposal() output tuple)
includes the lane field (same type as emitted in SlashProposed/SlashExecuted) or
add a dedicated getter like getSlashProposalLane(proposalId) that returns the
lane; modify the returned tuple name "proposal" from getSlashProposal and the
struct definition used by getSlashProposal to include the new lane member so
callers can distinguish Lane A vs B directly.
---
Nitpick comments:
In `@packages/enclave-contracts/test/BfvVkBindingIntegration.spec.ts`:
- Around line 235-247: The test currently calls the underlying ICircuitVerifier
(dkgCircuit.verify) which bypasses the wrapper logic; update the spec to include
at least one happy-path assertion that invokes the wrapper verify entrypoint
(e.g., call BfvPkVerifier.verify and/or BfvDecryptionVerifier.verify with the
decoded rawDkgProof and rawDkgPi or appropriately wrapped arguments) and expect
it to return true so the wrapper wiring and domain-binding are exercised; if you
want to keep the circuit-only assertion, either rename this test to indicate it
is "circuit-only" or split out a separate test that calls the wrapper verify
methods (BfvPkVerifier.verify / BfvDecryptionVerifier.verify) using the same
decoded inputs (rawDkgProof, rawDkgPi) and assert success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 47a77d80-6d00-45b7-9b22-36a5aa122703
📒 Files selected for processing (11)
packages/enclave-contracts/artifacts/contracts/Enclave.sol/Enclave.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.jsonpackages/enclave-contracts/artifacts/contracts/registry/CiphernodeRegistryOwnable.sol/CiphernodeRegistryOwnable.jsonpackages/enclave-contracts/artifacts/contracts/token/EnclaveTicketToken.sol/EnclaveTicketToken.jsonpackages/enclave-contracts/ignition/modules/bfvDecryptionVerifier.tspackages/enclave-contracts/ignition/modules/bfvPkVerifier.tspackages/enclave-contracts/scripts/utils.tspackages/enclave-contracts/test/BfvVkBindingIntegration.spec.ts
✅ Files skipped from review due to trivial changes (1)
- packages/enclave-contracts/ignition/modules/bfvDecryptionVerifier.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/enclave-contracts/contracts/lib/EnclavePricing.sol (1)
216-224: ⚡ Quick winClarify or remove unused
quotedFeefromvalidateRequest.
quotedFeeis documented as an input but unused in logic, which makes this API misleading for future payment-check refactors.♻️ Minimal clarity fix
- /// `@param` quotedFee Fee returned by {EnclavePricing.quote}. + /// `@param` _quotedFee Reserved for caller parity; currently unused. ... - uint256 quotedFee + uint256 _quotedFee🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/enclave-contracts/contracts/lib/EnclavePricing.sol` around lines 216 - 224, The validateRequest function currently accepts a quotedFee parameter that is never used; remove quotedFee from the validateRequest signature and its `@param` NatSpec to avoid a misleading API, and update any call sites or interfaces that pass quotedFee to validateRequest to stop doing so (search for validateRequest(...) usages and remove the extra argument). Ensure the function header and documentation only list the actual inputs (inputWindow, nowTs, computeWindow, decryptionWindow, maxDuration) so the contract API is accurate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/enclave-contracts/contracts/lib/EnclavePricing.sol`:
- Around line 216-224: The validateRequest function currently accepts a
quotedFee parameter that is never used; remove quotedFee from the
validateRequest signature and its `@param` NatSpec to avoid a misleading API, and
update any call sites or interfaces that pass quotedFee to validateRequest to
stop doing so (search for validateRequest(...) usages and remove the extra
argument). Ensure the function header and documentation only list the actual
inputs (inputWindow, nowTs, computeWindow, decryptionWindow, maxDuration) so the
contract API is accurate.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4910bb96-2cab-4bf4-9f98-c30acc4da59f
📒 Files selected for processing (25)
agent/flow-trace/00_INDEX.mdagent/flow-trace/04_DKG_AND_COMPUTATION.mdcircuits/benchmarks/results_insecure/crisp_verify_gas.jsoncircuits/benchmarks/results_insecure/integration_summary.jsonpackages/enclave-contracts/artifacts/contracts/Enclave.sol/Enclave.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IBondingRegistry.sol/IBondingRegistry.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/ICiphernodeRegistry.sol/ICiphernodeRegistry.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/IEnclave.sol/IEnclave.jsonpackages/enclave-contracts/artifacts/contracts/interfaces/ISlashingManager.sol/ISlashingManager.jsonpackages/enclave-contracts/artifacts/contracts/registry/CiphernodeRegistryOwnable.sol/CiphernodeRegistryOwnable.jsonpackages/enclave-contracts/contracts/Enclave.solpackages/enclave-contracts/contracts/interfaces/IEnclave.solpackages/enclave-contracts/contracts/lib/EnclavePricing.solpackages/enclave-contracts/test/BfvVkBindingIntegration.spec.tspackages/enclave-contracts/test/E3Lifecycle/E3Integration.spec.tspackages/enclave-contracts/test/E3Lifecycle/Sortition.spec.tspackages/enclave-contracts/test/Enclave.spec.tspackages/enclave-contracts/test/Governance/AccessAndBounds.spec.tspackages/enclave-contracts/test/Pricing/DustRotation.spec.tspackages/enclave-contracts/test/Pricing/Pricing.spec.tspackages/enclave-contracts/test/Pricing/PullPaymentsAndAllowlist.spec.tspackages/enclave-contracts/test/Registry/CiphernodeRegistryOwnable.spec.tspackages/enclave-contracts/test/Slashing/CommitteeExpulsion.spec.tspackages/enclave-contracts/test/fixtures/helpers.tspackages/enclave-contracts/test/fixtures/system.ts
💤 Files with no reviewable changes (11)
- packages/enclave-contracts/test/fixtures/helpers.ts
- packages/enclave-contracts/test/Enclave.spec.ts
- packages/enclave-contracts/test/Pricing/PullPaymentsAndAllowlist.spec.ts
- packages/enclave-contracts/test/E3Lifecycle/Sortition.spec.ts
- packages/enclave-contracts/test/Registry/CiphernodeRegistryOwnable.spec.ts
- packages/enclave-contracts/test/Pricing/DustRotation.spec.ts
- packages/enclave-contracts/test/Pricing/Pricing.spec.ts
- packages/enclave-contracts/test/fixtures/system.ts
- packages/enclave-contracts/test/Slashing/CommitteeExpulsion.spec.ts
- packages/enclave-contracts/contracts/interfaces/IEnclave.sol
- packages/enclave-contracts/test/E3Lifecycle/E3Integration.spec.ts
✅ Files skipped from review due to trivial changes (2)
- circuits/benchmarks/results_insecure/integration_summary.json
- agent/flow-trace/00_INDEX.md
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
agent/flow-trace/05_FAILURE_REFUND_SLASHING.md (1)
266-272:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a language tag to this fenced lifecycle block.
This block is missing a fence language (
MD040).Proposed doc fix
-``` +```text LIFECYCLE: Created by AccusationManagerExtension on SortitionCommitteeFinalized → Stores committee list, threshold_m, this node's address + signer → In-memory only (ephemeral — no persistence) → Destroyed by E3RequestComplete (Die signal)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@agent/flow-trace/05_FAILURE_REFUND_SLASHING.mdaround lines 266 - 272, The
fenced code block containing the "LIFECYCLE:" section is missing a language tag
(MD040); update the block surrounding the LIFECYCLE text (the triple-backtick
fence that opens before "LIFECYCLE:" and closes after "Die signal") to include a
language label such as "text" or "txt" (e.g., changetotext) so the
markdown linter no longer flags MD040 while leaving the block contents
unchanged.</details> </blockquote></details> </blockquote></details>🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In `@agent/flow-trace/03_E3_REQUEST_AND_COMMITTEE.md`: - Around line 310-314: The fenced code block showing "CiphernodeRegistrySolReader decodes SortitionCommitteeFinalized event" is missing a language tag (MD040); update the markdown fenced block in 03_E3_REQUEST_AND_COMMITTEE.md to include a language identifier such as "text" (e.g., ```text) so the block containing "CiphernodeRegistrySolReader" and "SortitionCommitteeFinalized" is properly tagged. - Around line 304-305: The event documentation for SortitionCommitteeFinalized currently lists only (e3Id, committee) but the runtime event mapping also includes scores; update the documentation to list the full payload (e3Id, committee, scores) and any brief descriptions for each field so the doc signature matches the runtime mapping and avoids reader/indexer confusion; locate the SortitionCommitteeFinalized mention and replace the short tuple with the complete tuple including scores and ensure any surrounding text that references the event uses the same three-field signature. --- Outside diff comments: In `@agent/flow-trace/05_FAILURE_REFUND_SLASHING.md`: - Around line 266-272: The fenced code block containing the "LIFECYCLE:" section is missing a language tag (MD040); update the block surrounding the LIFECYCLE text (the triple-backtick fence that opens before "LIFECYCLE:" and closes after "Die signal") to include a language label such as "text" or "txt" (e.g., change ``` to ```text) so the markdown linter no longer flags MD040 while leaving the block contents unchanged.🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID:
d7421a2d-a4df-4de3-8d8a-d939c4a2f4a5📒 Files selected for processing (4)
agent/flow-trace/03_E3_REQUEST_AND_COMMITTEE.mdagent/flow-trace/05_FAILURE_REFUND_SLASHING.mdcrates/evm/src/ciphernode_registry_sol.rscrates/evm/src/error_decoder.rs
Merge commit e82d4bd (feat: bind to on chain data #1544) into the contract-audit-fix branch. All conflicts resolved: - BfvPkVerifier / BfvDecryptionVerifier: merged new h/threshold constructor params and committee hash hi/lo circuit layout from main with HEAD's revert-on-failure semantics (InvalidProof, VkHashMismatch, etc.) and 6/8-param verify signatures. - Enclave.sol: kept 8-param decryptionVerifier.verify call with committeeHash sourced from ciphernodeRegistry.getCommitteeHash(e3Id). - CiphernodeRegistryOwnable: kept 6-param pkVerifier.verify call; removed caller-supplied nodes param from publishCommittee (now computed on-chain). - IDecryptionVerifier / IPkVerifier: kept HEAD's full NatSpec and custom errors. - Test specs rewritten to use new 6/8-param verify and main's circuit helpers. - utils.ts VkReader interfaces updated to camelCase immutable names. - fixtures/helpers.ts: removed nodes param from setupAndPublishCommittee. - Benchmark JSONs and flow-trace doc updated from main.
…c, zero-delay, docs) - Remove misleading uint256 totalClaimed return from claimRewards and claimSlashedFundsOnSuccessBatch: different e3Ids may use different fee tokens so summing amounts across denominations is meaningless. Callers should inspect per-E3 RewardClaimed / SlashedFundsClaimed events. - EnclaveToken: override _transferOwnership to synchronise AccessControl roles (DEFAULT_ADMIN_ROLE, MINTER_ROLE, WHITELIST_ROLE) on Ownable2Step handover. Without this the new owner() held no roles and the old owner silently retained full access. - ignition/modules/slashingManager.ts: add explicit warning comment that overriding initialDelay to 0 removes the two-step admin timelock (M-17). The default remains 2 days; the deployAndSave path already enforces > 0. - agent/flow-trace: fix SortitionCommitteeFinalized call-site to include the scores parameter; add 'text' language tag to bare code fence.
ctrlc03
approved these changes
May 21, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Critical
enableE3Programis intended).proofAggregationEnabled == falsepath is intentional fornon-aggregating programs).
ExitQueueLib._takeAssetsFromQueuenow keeps two independent heads per asset class somixed tranches no longer strand the other asset.
unnecessary for the target deployment).
GOVERNANCE_ROLEis now admin ofSLASHER_ROLE;DEFAULT_ADMIN_ROLEcan no longerself-grant slashing.
AccessControlDefaultAdminRulesprovides a 2-day delay on admin handover; per-settertimelock deferred to governance contract.
AccessControlDefaultAdminRules) +Ownable2Stepon every privilegedcontract; multisig wiring is a deployment-time concern.
BfvPkVerifier/BfvDecryptionVerifieradd a wrapper-level domain-binding slot(
chainId | verifyingContract | e3Id); full circuit-level enforcement tracked as future work(Noir circuits are off-limits in this PR).
High
_pendingRewards; one blacklisted recipient cannotDoS the batch.
EnclaveTicketTokenconstructor no longer routes throughonlyOwner setRegistry;deployment succeeds when
deployer != initialOwner_.depositFor/depositFromuse balance-delta accounting to defeat fee-on-transfer /rebasing base tokens.
_validateNodeEligibilityderives weight fromgetTicketBalanceAtBlock(operator, requestBlock - 1)(ERC20Votes snapshot), so post-requesttop-ups can't inflate selection weight.
deregisterOperatoris gated byhasOpenLaneBProposal; Lane B counter preventsclaim-exits race.
appealWindow > 0, giving the honest minoritytime to dispute.
withdrawSlashedFunds(pull-payment); late-credit handledeven if operator deregistered.
calculateRefundredirects the honest-node share to the requester whenhonestNodes.length == 0; same fix in_applySlashedFunds._pendingSlashedFundsis re-snapshotted on_applySlashedFundsand final-claim dustflows to
_pendingTreasury; no orphaning.EnclaveSlashing/1domain) includingverifyingContract+chainId.Ownableupgraded toOwnable2Step(Enclave / CiphernodeRegistry /BondingRegistry / E3RefundManager / EnclaveToken / EnclaveTicketToken).
MAX_APPEAL_WINDOW = 30 days;SlashPolicy.enabledis now toggleable both directions.MAX_PROTOCOL_SHARE_BPS = 5_000cap enforced onsetPricingConfig.MAX_PROTOCOL_BPS = 5_000cap onE3RefundManager.setWorkAllocation.token-distribution concern.
EnclaveTicketTokenclock istimestamp(EIP-6372); registry repoint no longerrewrites checkpoints same-block.
MAX_TIMEOUT_WINDOW = 30 daysandMAX_COMMITTEE_SIZE = 256enforced onsetTimeoutConfig/setCommitteeThresholds.MIN_EXIT_DELAY = 1 day,MAX_EXIT_DELAY = 90 daysonBondingRegistry.setExitDelay.setSlashingManagerrejects zero address;setPkVerifiersimilarly guarded.underlying.
MAX_EXITS_PER_OPERATORbound + slasher OOG defence via deferred processing.scripts/validateUpgrade.ts(solcstorageLayoutdiff vsaudits/storage-layouts/*.jsonsnapshots) wired aspnpm validate:upgrade; OZhardhat-upgradesplugin not Hardhat-3 compatible.
scripts/deployEnclave.tsrejects mock deployments outside dev chain-ids(
31337 / 1337 / 11155111 / 5 / 80001); override viaALLOW_MOCKS_ON_PRODUCTION.SLASHER_ROLEtoGOVERNANCE_ROLE.Committee.requestBlock = block.timestampso it aligns with the ERC20Votes EIP-6372timestamp clock.
evmVersion: "paris"+metadata.bytecodeHash: "none"pinned inhardhat.config.ts.PkVerifierSet(bytes32 indexed schemeId, address indexed verifier)event emitted.Medium
nonReentrantonEnclave.request(pluspublishCiphertextOutput,publishPlaintextOutput,processE3Failure, all claim paths)._pendingTreasury.SafeERC20.forceApproveand resets to zero after batch.MAX_CIPHERNODE_LEAVES = 1 << TREE_DEPTH;addCiphernodereverts withCiphernodeTreeExhausted()past capacity.MAX_SORTITION_SUBMISSION_WINDOWbound onsetSortitionSubmissionWindow.getE3QuoteusesMath.mulDivend-to-end for duration math.e3Id % committeeSize.setExitDelayonly affects future tranches; monotonic-unlock invariant preserved.RefundDistribution.perNodeAmountsnapshotted once, re-snapshotted on_applySlashedFunds, last-claim dust →_pendingTreasury.Enclave._feeTokenAllowedmapping +setFeeTokenAllowedadmin gate; only whitelistedERC-20s accepted.
EnclaveTicketToken.permitoverridden to revert (SoulboundPermitDisabled).rescueERC20(owner-only) onEnclaveTicketToken.BondingRegistry.bondLicense/ exit paths now share delta-accounting helper.SlashPolicy.enabledtoggleable both directions.proposeBan/confirmBan) with distinct signers required.renounceOwnership()overridden to revert withRenounceOwnershipDisabledon everyOwnable contract.
AccessControlDefaultAdminRuleswith 2-day handover delay.Enclave.requestacceptsmaxFee; reverts withMaxFeeExceededif quote drifts.PkVerifierSet(bytes32 indexed schemeId, address indexed verifier)emitted.BondingRegistryUpdated,RefundManagerUpdated,FeeTokenAllowedSet,MarkFailedGracePeriodSet, etc.).MINTER_ROLEis already gated byAccessControlDefaultAdminRules).EnclaveTicketToken.setRegistryis two-step with delay; can't instant-flipcheckpoints.
MAX_DURATION_CAP = 365 daysandMAX_COMMITTEE_SIZE = 256enforced.EnclaveTicketTokenoverridesdelegate/delegateBySigto auto-self-delegate tothe holder (no
IVotesrevert).uint256[50] private __gapappended toEnclave,CiphernodeRegistryOwnable,BondingRegistry,E3RefundManager. Storage layouts snapshotted underaudits/storage-layouts/and enforced by
pnpm validate:upgrade._owner.clock()=timestamp,CLOCK_MODE()="mode=timestamp"on both tokens.concern.
evmVersion=parisaligns with non-zkSync EVMs.markE3Failedrestricted to requester / committee / owner duringmarkFailedGracePeriod; permissionless after. Setter isEnclave.setMarkFailedGracePeriod(uint256)(owner-only, emitsMarkFailedGracePeriodSet).BfvPkVerifier/BfvDecryptionVerifierpin sub-circuit VK hashes asimmutable.reverton bad proof instead of returning false.SlashProposed/SlashExecutedcarry alanefield.IEnclave.InputPublishedremoved (was dead).ICiphernodeRegistry.CommitteeFinalizedrenamed toSortitionCommitteeFinalized; nocollision with
IEnclave.CommitteeFinalized.Low / Informational
PausabletoEnclave.solwould push it back overEIP-170; revisit after a deeper library split.
setParamSetoverwrite emitsParamSetUpdated(prev, new)(first call still emitsParamSetRegistered).BondingRegistryenforcesMAX_AUTHORIZED_DISTRIBUTORS = 32; reverts withMaxAuthorizedDistributorspast cap.supportsInterfaceimplemented onEnclave,CiphernodeRegistryOwnable,BondingRegistry,E3RefundManager,SlashingManager.signatures are validated off-chain via
ECDSA.recover+SLASHER_ROLE); EIP-1271 would be deadcode.
EnclaveTicketToken/EnclaveTokenadvertiseclock()andCLOCK_MODE()("mode=timestamp").
forceApprove(0)redundancy cleaned, 18 silent setters now emit events (coveredby M-20), all listed
L-*andI-*items either folded into a Medium fix above or out-of-scopeper audit (SDK / indexer drift tracked separately).
Summary by CodeRabbit
Release Notes
New Features
Improvements