feat(pool): add MARKPool ZK UTXO pool domain#100
Merged
Conversation
Introduces the full pool domain for private RYLA transfers:
Contracts:
- MARKPool: ZK UTXO pool with Merkle tree, fee policy, bridge-out/in,
withdraw binding, AccessManaged access control
- MARKWithdrawAdapter: EIP-712 signature-based withdrawal adapter
- RYLACreditLedger: ICreditLedger adapter bridging MARKPool to RYLA
mint/burn; restricted to pool caller only (onlyPool)
- PoolFeePolicy, PoolPublicInputs, PoolValidation: pool support libraries
- MARKPoolVerifier: Groth16 verifier generated from MARKPool circuit
(13 public signals, pot15 trusted setup)
Interfaces: ICreditLedger, IVerifier, IPoolBridge, IPoolNullifier
Crypto: MerkleTree (Poseidon, depth-20), ProofUtils, PoseidonT3
Circuit:
- circuits/mark/MARKPool.circom: MARK-native UTXO circuit (depth=20,
2-in/2-out, 13 public signals); renamed from prototype utxo.circom,
domain constants documented as permanent, hardcoded fee policy removed
- circuits/setup.mjs: trusted setup script (pot15)
- circuits/test/MARKPool.test.mjs: 13 witness tests
CI: circuits-ci.yml runs witness tests on every PR
Tests: MARKPool.t.sol (22), MARKWithdrawAdapter.t.sol (9),
RYLACreditLedger.t.sol (8)
- PoolErrors.sol: rewrite to match Pool.sol, PoolValidation.sol, and MerkleTree.sol — adds 25 missing errors (build was broken), removes 18 errors only used by the old MARKPool prototype - MARKPool.sol: rename domain separator Pool.WithdrawBinding.v1 to MARKPool.WithdrawBinding.v1 (permanent, must be set before deploy) - MARKWithdrawAdapter.sol: rename domain separator WithdrawAdapter.Intent.v1 to MARKWithdrawAdapter.Intent.v1 - UTXOVerifier.sol: delete (built for old 4-signal circuit, wrong interface, superseded by MARKPoolVerifier.sol) - IUTXOVerifier.sol: delete (superseded by IVerifier.sol) - UTXOSettlement.circom: delete (superseded by MARKPool.circom) - Groth16SettlementVerifier.sol: update stale comment - KNOWN_ISSUES.md: add KI-7 (two-circuit architecture), KI-8 (pool domain access control model) - foundry.toml: via_ir = true for pool domain compilation
Dependency ReviewThe following issues were found:
License Issues.github/workflows/circuits-ci.yml
OpenSSF Scorecard
Scanned Files
|
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
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:
WalkthroughAdds a new MARKPool ZK-UTXO flow: a 13-signal Circom circuit, witness tests and trusted-setup, a regenerated Groth16 verifier, Merkle/Proof utilities, a proof-driven on-chain MARKPool with root/ nullifier management, RYLA credit ledger, MARKWithdrawAdapter, extensive tests, deploy/release scripts, CI workflows, and docs/guard updates. ChangesCircuits, build, trusted-setup & witness testing
Merkle/verifier primitives, pool contracts, withdraw flow, infra and tests
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Offchain as Witness
participant Pool as MARKPool
participant Verifier as MARKPoolVerifier
participant Tree as MerkleTree
participant Ledger as RYLACreditLedger
User->>Offchain: build proof (nullifiers,outCommitments,merkleRoot,fee,...)
Offchain-->>User: proof + public signals
User->>Pool: transact(merkleRoot,nullifiers,outCommitments,fee,relayer,proof)
Pool->>Pool: validate ranges & nullifier freshness
Pool->>Verifier: verifyProof(publicInputs, proof)
Verifier-->>Pool: proof valid
Pool->>Pool: mark nullifiers used
Pool->>Tree: insert(outCommitment[0])
Pool->>Tree: insert(outCommitment[1])
Tree-->>Pool: root updated
Pool->>Ledger: credit(relayer, relayerAmount)
Ledger-->>Pool: credited
Pool-->>User: transaction accepted
sequenceDiagram
participant Owner as Owner
participant Intent as IntentSigner
participant Adapter as MARKWithdrawAdapter
participant Pool as IPoolNullifier
participant Ledger as RYLACreditLedger
participant Recipient as Recipient
Owner->>Owner: sign owner signature
Intent->>Intent: sign intent signature
User->>Adapter: withdrawWithSig(nullifiers, ownerSig, intentSig, nonce, deadline)
Adapter->>Pool: isNullifierUsedGlobal(nullifier)
Pool-->>Adapter: not used
Adapter->>Pool: nullifierWithdrawBinding(nullifier)
Pool-->>Adapter: binding hash
Adapter->>Adapter: verify signatures, nonce, liquidity
Adapter->>Adapter: mark nullifiers claimed, increment nonce
Adapter->>Ledger: debit(creditOwner, amount)
Ledger-->>Adapter: debited
Adapter->>Recipient: transfer ETH
Recipient-->>Adapter: received
Estimated code review effort🎯 5 (Critical) | ⏱️ ~110 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 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 `@circuits/package.json`:
- Around line 7-8: The test script runs before the circom build so CI fails with
MODULE_NOT_FOUND for build/MARKPool_js/witness_calculator.js; update the
package.json scripts so the build runs before tests—either add a "pretest"
script that runs the existing "build" (or change "test" to run "npm run build &&
node test/MARKPool.test.mjs") so that the MARKPool.circom artifacts are
generated prior to invoking MARKPool.test.mjs; update the scripts referencing
"build" and "test" accordingly.
In `@contracts/KNOWN_ISSUES.md`:
- Around line 77-102: KI-7 is duplicated and contains stale/conflicting details
(absolute path, mixed legacy circuit names); consolidate into one canonical
entry that references the current symbols: Pool, MARKSettlementModule,
Groth16SettlementVerifier, AttestedSettlementVerifier, and utxo.circom; remove
the duplicate KI number and any mention of UTXOSettlement.circom except as an
explicit legacy note, replace the absolute artifact path with a repo-relative
path (e.g., ./circuits/artifacts/prod/), and make a single authoritative
Status/Impact block that states the pool and verifier use the 13-signal layout
(list merkleRoot, chainId, dstChainId, protocolEpoch, fee, relayer,
nullifier[2], outCommitment[2], withdrawOwner, withdrawRecipient,
withdrawAmount), note that Groth16SettlementVerifier still needs the generated
verifier contract integrated, and keep AttestedSettlementVerifier described as
the signature fallback.
In `@contracts/src/pool/MARKPool.sol`:
- Around line 345-356: bridgeIn lacks replay protection: add a unique source
message identifier parameter (e.g., bytes32 srcMessageId) to bridgeIn, record
processed messages on-chain and reject duplicates before calling
_insertCommitments; implement a storage mapping to track processed messages
keyed by srcChainId and srcMessageId, revert with a clear error (e.g.,
AlreadyProcessed) if the id was seen, mark the id processed after successful
insertion, and include the srcMessageId in the BridgeIn event so relayers can
correlate deliveries and retries.
In `@contracts/src/pool/PoolFeePolicy.sol`:
- Around line 6-12: Add explicit precondition checks at the start of the split
function to prevent panic-reverts: require that maxFeeBurnBps != 0 and that
feeBurnBps <= maxFeeBurnBps, each with a descriptive revert message (e.g.,
"maxFeeBurnBps>0" and "feeBurnBps<=maxFeeBurnBps"); then proceed with the
existing arithmetic and return burnAmount and relayerAmount as before.
In `@contracts/src/settlement/verifier/Groth16SettlementVerifier.sol`:
- Around line 13-14: Update the NatSpec comment in Groth16SettlementVerifier so
the circuit name is consistent: replace any mention of "UTXOSettlement circuit"
with the finalized circuit name "MARKPool" (or use "MARKPool circuit") and
ensure the example verifier name MARKPoolVerifier and the interface
IUTXOSettlementVerifier provenance are both referenced consistently in the
docblock; update the two lines to state that this contract delegates to a
Groth16 verifier (e.g. MARKPoolVerifier) generated from the MARKPool circuit so
integrators have an unambiguous circuit/verifier mapping.
- Around line 84-93: The isMint parameter is currently ignored (no-op) which
allows wrong-direction settlements; update the verifier to fail-closed by
explicitly validating isMint in the verifier entrypoint (the symbol isMint in
the Groth16SettlementVerifier logic) — e.g., require the value equals the
expected direction for settlement (reject/more strictly: revert when isMint
indicates a mint) until a dedicated direction public signal or separate verifier
path exists; add a clear revert message referencing isMint to aid debugging.
In `@contracts/src/withdraw/MARKWithdrawAdapter.sol`:
- Around line 111-120: The current computeWithdrawIntentDigest uses
MessageHashUtils.toEthSignedMessageHash (EIP-191/personal_sign) which is
incompatible with EIP-712; replace this pattern by inheriting OpenZeppelin's
EIP712 and compute the digest via _hashTypedDataV4(structHash) instead of
toEthSignedMessageHash, then use ECDSA.recover(...) when verifying signatures;
update computeWithdrawIntentDigest (and the analogous logic referenced around
lines 208–215, e.g., any functions calling computeWithdrawIntentHash or
performing signature recovery) to build the typed-data structHash with
computeWithdrawIntentHash, call _hashTypedDataV4(structHash) to get the
domain-aware digest, and use ECDSA.recover for verification so clients using
EIP-712 will produce matching signatures.
- Line 147: The adapter currently calls assetLedger.debit(creditOwner, amount)
directly, but RYLACreditLedger.debit() is protected by onlyPool so the adapter
will be Unauthorized(); either route the debit through the Pool or grant the
adapter pool-level authorization: modify MARKWithdrawAdapter to call the Pool
contract to perform the debit (e.g., invoke a Pool method that in turn calls
assetLedger.debit) instead of calling assetLedger.debit directly, or update
RYLACreditLedger access control to allow this adapter (or add an
adapter-specific entrypoint) so that calls originate from the POOL-authorized
context; target references: assetLedger.debit, RYLACreditLedger.debit, onlyPool
and the POOL address.
In `@contracts/test/unit/pool/MARKPool.t.sol`:
- Around line 292-296: Replace the broad vm.expectRevert() in
testBridgeInRevertsWhenCallerNotRestricted with an expectation for the pool
contract's specific revert selector/error (the error emitted when non-restricted
callers call bridgeIn). Update the test to call vm.expectRevert(...) with
abi.encodeWithSelector(TheContractSpecificError.selector, /* args if any */)
before calling pool.bridgeIn(901, commitments) so the test asserts the exact
revert reason from bridgeIn rather than any revert.
In `@contracts/test/unit/withdraw/MARKWithdrawAdapter.t.sol`:
- Around line 156-182: The three placeholder tests must actually invoke the
adapter withdrawal path and assert outcomes: in
testWithdrawRequiresSufficientLiquidity deploy MARKWithdrawAdapter and call
adapter.withdraw(...) (or the actual withdraw-entry method) with valid dummy
params under a vm.prank(user) and assert it reverts with "Insufficient
liquidity" when the adapter has 0 balance; in testWithdrawNonceIncrement fund
the adapter with native tokens, perform a successful adapter.withdraw(...) and
then assert adapter.withdrawNonce(user) == 1; in
testNullifierCannotBeClaimedTwice call adapter.withdraw(...) (or the internal
claim method) once with a chosen nullifier and assert
adapter.claimedNullifiers(nullifier) is true, then attempt the same
withdraw/claim again and assert it reverts (duplicate nullifier). Use the
existing MARKWithdrawAdapter instance, adapter.withdraw (or the contract’s
public withdraw method), adapter.withdrawNonce, and adapter.claimedNullifiers to
locate the code to modify.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b075a90-6f6e-49cd-85c1-f7bd952b3edf
⛔ Files ignored due to path filters (1)
contracts/src/crypto/generated/PoseidonT3.solis excluded by!**/generated/**
📒 Files selected for processing (29)
.github/workflows/circuits-ci.ymlcircuits/.gitignorecircuits/mark/MARKPool.circomcircuits/package.jsoncircuits/setup.mjscircuits/test/MARKPool.test.mjscircuits/utxo/UTXOSettlement.circomcontracts/KNOWN_ISSUES.mdcontracts/foundry.tomlcontracts/src/crypto/MerkleTree.solcontracts/src/crypto/ProofUtils.solcontracts/src/interfaces/ICreditLedger.solcontracts/src/interfaces/IPoolBridge.solcontracts/src/interfaces/IPoolNullifier.solcontracts/src/interfaces/IVerifier.solcontracts/src/pool/MARKPool.solcontracts/src/pool/PoolFeePolicy.solcontracts/src/pool/PoolPublicInputs.solcontracts/src/pool/PoolValidation.solcontracts/src/pool/RYLACreditLedger.solcontracts/src/pool/errors/PoolErrors.solcontracts/src/pool/interfaces/IUTXOVerifier.solcontracts/src/pool/verifier/MARKPoolVerifier.solcontracts/src/settlement/verifier/Groth16SettlementVerifier.solcontracts/src/withdraw/MARKWithdrawAdapter.solcontracts/src/withdraw/MARKWithdrawErrors.solcontracts/test/unit/pool/MARKPool.t.solcontracts/test/unit/pool/RYLACreditLedger.t.solcontracts/test/unit/withdraw/MARKWithdrawAdapter.t.sol
💤 Files with no reviewable changes (2)
- contracts/src/pool/interfaces/IUTXOVerifier.sol
- circuits/utxo/UTXOSettlement.circom
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 02c9da0121
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- MARKPool, MARKWithdrawAdapter: rename immutables to SCREAMING_SNAKE_CASE (assetLedger->ASSET_LEDGER, proofPool->PROOF_POOL) - MARKPool: remove _assetLedger from constructor; add setAssetLedger() one-time restricted setter to break circular deploy dependency with RYLACreditLedger - DeployMARKPool.s.sol: full deployment script for pool domain (AccessManager, MARKPool, RYLACreditLedger, MARKWithdrawAdapter) - MARKPool.sol: add withdrawal flow NatSpec (burn-to-claim model) - ARCHITECTURE.md: add pool/withdraw domains, dependency rules, and withdrawal flow section - MARKPoolInvariants.t.sol: 3 invariants (nullifiers never unspent, withdraw bindings immutable, root queue only grows) - architecture-guard.sh: add pool->settlement/bridge and withdraw->settlement/bridge isolation rules
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
contracts/src/pool/MARKPool.sol (1)
379-388:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
bridgeInstill lacks explicit replay protection for delivered bridge messages.This path has no
srcMessageId/processed-message guard, so duplicate relay deliveries can be accepted again.🤖 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 `@contracts/src/pool/MARKPool.sol` around lines 379 - 388, The bridgeIn function currently accepts duplicate deliveries because it lacks a processed-message guard; update bridgeIn to accept a unique srcMessageId (e.g., add a bytes32 or uint256 srcMessageId parameter), check a processed mapping (e.g., processedBridgeMessages[srcChainId][srcMessageId]) and revert on replay, mark the srcMessageId as processed before calling _insertCommitments(outCommitments), and then emit BridgeIn as before; ensure you add the storage mapping and any accessor/helper (processedBridgeMessages) used to record and check processed messages.contracts/src/withdraw/MARKWithdrawAdapter.sol (2)
111-121:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftIntent digest path is EIP-191 (
personal_sign), not EIP-712 typed data.Line 120 and Line 209 use
toEthSignedMessageHash, so EIP-712 clients will produce non-matching signatures despite the typed-intent semantics.#!/bin/bash set -euo pipefail echo "== Adapter digest code ==" sed -n '85,220p' contracts/src/withdraw/MARKWithdrawAdapter.sol echo echo "== OpenZeppelin MessageHashUtils semantics ==" fd -i 'MessageHashUtils.sol' | xargs -r sed -n '1,220p' | rg -n 'toEthSignedMessageHash|Ethereum Signed Message' echo echo "== OpenZeppelin EIP712 typed-data helper ==" fd -i 'EIP712.sol' | xargs -r rg -n '_hashTypedDataV4'Also applies to: 208-215
🤖 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 `@contracts/src/withdraw/MARKWithdrawAdapter.sol` around lines 111 - 121, The contract is using OpenZeppelin's MessageHashUtils.toEthSignedMessageHash (EIP-191) for computeWithdrawIntentDigest (and the other usage around the withdraw verification logic), but the intent is typed data (EIP-712); replace the toEthSignedMessageHash call with the EIP-712 domain+typed-data flow by computing the struct hash for the withdraw intent (use a WITHDRAW_INTENT_TYPEHASH and computeWithdrawIntentHash or build keccak256(abi.encode(WITHDRAW_INTENT_TYPEHASH, creditOwner, recipient, amount, nullifiers, nonce, deadline))) and then call _hashTypedDataV4(structHash) from the EIP712 base (instead of MessageHashUtils.toEthSignedMessageHash). Update computeWithdrawIntentDigest and any other places that call toEthSignedMessageHash (e.g., the verify/validation code that reads the intent digest) to use _hashTypedDataV4 so signatures match EIP-712 typed-data semantics.
147-147:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift
withdrawWithSigcan hard-revert on ledger debit authorization.Line 147 calls
ASSET_LEDGER.debitdirectly, but this PR’s ledger model is pool-restricted. If the ledger enforces pool-only caller checks, all withdrawals fail at debit.🤖 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 `@contracts/src/withdraw/MARKWithdrawAdapter.sol` at line 147, withdrawWithSig currently calls ASSET_LEDGER.debit directly which will hard-revert under the pool-restricted ledger model; replace the direct debit with the ledger’s pool-authorized debit API (e.g. ASSET_LEDGER.debitFromPool or ASSET_LEDGER.debitFrom(poolAddress/ poolId, creditOwner, amount)) or route the debit through the Pool contract (e.g. POOL.performDebit/ poolAuthorizedDebit) so the ledger sees an authorized pool caller; update withdrawWithSig to use that pool-authorized method and pass the adapter’s pool identifier/address when invoking the ledger.
🤖 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 `@contracts/script/ci/architecture-guard.sh`:
- Around line 47-54: The existing forbidden-import regex used in the
check_no_imports calls (the pattern starting
'^import\s+.*"(?:\.\.\/settlement\/|\.\.\/bridge\/|src\/settlement\/|src\/bridge\/)')
doesn't match nested paths like ../../src/settlement or ../../settlement; update
both regex occurrences passed to check_no_imports (the pool -> settlement/bridge
and withdraw -> settlement/bridge checks) to allow any number of "../" segments
and an optional "src/" prefix before "settlement" or "bridge" (e.g. use a
pattern that accepts (?:\.\.\/)+ and optional src/ before (?:settlement|bridge)
), so imports like ../../src/settlement/... are caught.
In `@contracts/script/deploy/pool/DeployMARKPool.s.sol`:
- Around line 60-77: The deployment broadcasts as cfg.deployer but constructs
AccessManager with cfg.owner and then makes admin-only calls (e.g.,
d.accessManager.grantRole(POOL_ADMIN_ROLE,...)) as cfg.deployer, causing reverts
when cfg.owner != cfg.deployer; fix by executing the admin-scoped operations
under cfg.owner (either wrap the admin sequence that interacts with
AccessManager — including grantRole on AccessManager and any subsequent admin
calls related to AccessManager, MARKPool, RYLACreditLedger, MARKWithdrawAdapter
setup — in a vm.prank(cfg.owner) or switch the broadcast to cfg.owner for that
section) so the AccessManager sees msg.sender == cfg.owner during those calls.
In `@contracts/src/pool/MARKPool.sol`:
- Around line 465-472: Add an explicit guard in _applyFee to ensure ASSET_LEDGER
is configured before it's used: check that address(ASSET_LEDGER) != address(0)
(and revert with a clear message like "Asset ledger not set") before calling
ASSET_LEDGER.credit or emitting FeePaid; place this check early in _applyFee
(e.g., immediately after the fee==0 return or before the relayerAmount > 0
branch) so attempts to use ASSET_LEDGER prior to setAssetLedger() fail with a
readable error instead of an opaque external-call revert.
---
Duplicate comments:
In `@contracts/src/pool/MARKPool.sol`:
- Around line 379-388: The bridgeIn function currently accepts duplicate
deliveries because it lacks a processed-message guard; update bridgeIn to accept
a unique srcMessageId (e.g., add a bytes32 or uint256 srcMessageId parameter),
check a processed mapping (e.g.,
processedBridgeMessages[srcChainId][srcMessageId]) and revert on replay, mark
the srcMessageId as processed before calling _insertCommitments(outCommitments),
and then emit BridgeIn as before; ensure you add the storage mapping and any
accessor/helper (processedBridgeMessages) used to record and check processed
messages.
In `@contracts/src/withdraw/MARKWithdrawAdapter.sol`:
- Around line 111-121: The contract is using OpenZeppelin's
MessageHashUtils.toEthSignedMessageHash (EIP-191) for
computeWithdrawIntentDigest (and the other usage around the withdraw
verification logic), but the intent is typed data (EIP-712); replace the
toEthSignedMessageHash call with the EIP-712 domain+typed-data flow by computing
the struct hash for the withdraw intent (use a WITHDRAW_INTENT_TYPEHASH and
computeWithdrawIntentHash or build
keccak256(abi.encode(WITHDRAW_INTENT_TYPEHASH, creditOwner, recipient, amount,
nullifiers, nonce, deadline))) and then call _hashTypedDataV4(structHash) from
the EIP712 base (instead of MessageHashUtils.toEthSignedMessageHash). Update
computeWithdrawIntentDigest and any other places that call
toEthSignedMessageHash (e.g., the verify/validation code that reads the intent
digest) to use _hashTypedDataV4 so signatures match EIP-712 typed-data
semantics.
- Line 147: withdrawWithSig currently calls ASSET_LEDGER.debit directly which
will hard-revert under the pool-restricted ledger model; replace the direct
debit with the ledger’s pool-authorized debit API (e.g.
ASSET_LEDGER.debitFromPool or ASSET_LEDGER.debitFrom(poolAddress/ poolId,
creditOwner, amount)) or route the debit through the Pool contract (e.g.
POOL.performDebit/ poolAuthorizedDebit) so the ledger sees an authorized pool
caller; update withdrawWithSig to use that pool-authorized method and pass the
adapter’s pool identifier/address when invoking the ledger.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 127a5593-2d3f-46b8-8269-88eb97907e79
📒 Files selected for processing (7)
contracts/ARCHITECTURE.mdcontracts/script/ci/architecture-guard.shcontracts/script/deploy/pool/DeployMARKPool.s.solcontracts/src/pool/MARKPool.solcontracts/src/withdraw/MARKWithdrawAdapter.solcontracts/test/invariant/pool/MARKPoolInvariants.t.solcontracts/test/unit/pool/MARKPool.t.sol
- DeployMARKPool.s.sol: grant POOL_ADMIN_ROLE to deployer during setup so setAssetLedger/setIntentSigner calls succeed when deployer != owner; revoke deployer role after setup completes - MARKPool._applyFee: revert InvalidAssetLedger if ASSET_LEDGER is not set and a non-zero fee is applied (prevents silent call to address(0))
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@contracts/src/pool/MARKPool.sol`:
- Around line 220-228: The FixedFeePolicy error used in setMinFee is ambiguous
given the function constrains newMinFee to 0 or 1; either rename the error to
something descriptive like MinFeeTooLarge (or MinFeeExceedsMaximum) and update
all references, or add a clarifying NatSpec/comment to the FixedFeePolicy error
definition explaining it means "min fee must be 0 or 1". Update the revert in
setMinFee (and any other uses) to use the new error name or keep the same name
but ensure the error's definition includes the explanatory note; refer to
setMinFee, minFee, FixedFeePolicy and MinFeeSet when making the changes.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 21cd4d11-ba48-4a30-bf5a-dd54e6c4b7e4
📒 Files selected for processing (2)
contracts/script/deploy/pool/DeployMARKPool.s.solcontracts/src/pool/MARKPool.sol
circuits/build/ is gitignored so the WASM and witness_calculator.js are not in the repo. Add circom install and npm run build steps before npm test so CI compiles the circuit fresh on each run.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/circuits-ci.yml:
- Around line 33-35: The workflow currently writes circom to /usr/local/bin
using the commands "curl -L ... -o /usr/local/bin/circom" and "chmod +x
/usr/local/bin/circom", which can fail due to permission errors; update the step
to either (A) run the install commands with elevation (prefix curl and chmod
with sudo) or (B) install into a user-writable location like $HOME/.local/bin
(create it with mkdir -p $HOME/.local/bin, download circom there, chmod +x, and
export PATH="$HOME/.local/bin:$PATH") before running "circom --version" so the
runner can write the file without root permission issues.
- Around line 23-27: The CI step named "Setup Node.js" currently pins
node-version '20' using the actions/setup-node@v6 action; update the workflow to
use Node.js 24 by changing the node-version value to '24' (keep the step name
"Setup Node.js" and the same action unless you also want to update the action
version), ensuring the GitHub Actions runner will use the Active LTS Node
runtime.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: c0042e34-e542-4997-9525-f7dbbdc5bc57
📒 Files selected for processing (1)
.github/workflows/circuits-ci.yml
- Rename immutables to SCREAMING_SNAKE_CASE: assetLedger->ASSET_LEDGER, proofPool->PROOF_POOL (MARKPool.sol, MARKWithdrawAdapter.sol) - MARKPool: remove _assetLedger from constructor, add setAssetLedger() one-time restricted setter to break circular deploy dependency with RYLACreditLedger - MARKPool: add withdrawal flow documentation to contract NatSpec - ARCHITECTURE.md: add pool/withdraw domains, dependency rules, and withdrawal flow explanation - DeployMARKPool.s.sol: deployment script for MARKPool, RYLACreditLedger, MARKWithdrawAdapter with AccessManager configuration - MARKPoolInvariants.t.sol: 3 invariants (nullifiers never unspent, withdraw bindings immutable, root queue only grows) - architecture-guard.sh: add pool and withdraw domain isolation rules
- circuits-ci.yml: updated to run MARKPool witness tests - circuits/package.json: build/test scripts point to MARKPool.circom - circuits/setup.mjs: updated for MARKPool.circom trusted setup - circuits/test/MARKPool.test.mjs: cleaned up test file - contracts/KNOWN_ISSUES.md: updated KI-7 for current two-circuit state - contracts/src/pool/errors/PoolErrors.sol: add missing blank line
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
contracts/src/pool/MARKPool.sol (1)
72-73:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope
bridgeInreplay protection by source chain.At Line 390–Line 391, replay protection is keyed only by
messageId. If two source chains emit the same ID format, one can block the other. Key replay state by(srcChainId, messageId).Suggested fix
- mapping(bytes32 => bool) public processedBridgeMessages; + mapping(bytes32 => bool) public processedBridgeMessages; @@ - if (processedBridgeMessages[messageId]) revert BridgeMessageAlreadyProcessed(); - processedBridgeMessages[messageId] = true; + bytes32 replayKey = keccak256(abi.encode(srcChainId, messageId)); + if (processedBridgeMessages[replayKey]) revert BridgeMessageAlreadyProcessed(); + processedBridgeMessages[replayKey] = true;Also applies to: 390-391
🤖 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 `@contracts/src/pool/MARKPool.sol` around lines 72 - 73, The replay-protection mapping processedBridgeMessages is only keyed by messageId which allows collisions across source chains; update the logic to scope replay state by (srcChainId, messageId) instead: change the mapping key usage in the bridgeIn processing and any reads/writes (references: processedBridgeMessages and any place setting/checking it, e.g., inside bridgeIn) to use a composite key like keccak256(abi.encodePacked(srcChainId, messageId)), and do the same for nullifierWithdrawBinding if it is tied to bridge messages; ensure all checks and assignments that previously used messageId alone now use the composite key so different srcChainIds cannot conflict.contracts/test/unit/pool/MARKPool.t.sol (1)
294-297:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the exact revert in unauthorized
bridgeIntest.Line 296 uses a broad
vm.expectRevert(), which can pass on unrelated failures. Assert the specific AccessManaged unauthorized selector instead.🤖 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 `@contracts/test/unit/pool/MARKPool.t.sol` around lines 294 - 297, Replace the broad vm.expectRevert() in testBridgeInRevertsWhenCallerNotRestricted with an assertion for the specific AccessManaged unauthorized error: call vm.expectRevert(abi.encodeWithSelector(AccessManaged.Unauthorized.selector)) before invoking pool.bridgeIn(...) so the test only passes for the intended unauthorized revert coming from the AccessManaged contract..github/workflows/circuits-ci.yml (1)
30-31:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInstall
circomin a user-writable path (or usesudo).Line 30 writes to
/usr/local/bindirectly; this commonly fails on GitHub-hosted runners due to permissions.Suggested fix
- name: Install circom run: | - curl -L https://github.com/iden3/circom/releases/latest/download/circom-linux-amd64 -o /usr/local/bin/circom - chmod +x /usr/local/bin/circom + mkdir -p "$HOME/.local/bin" + curl -fsSL https://github.com/iden3/circom/releases/latest/download/circom-linux-amd64 -o "$HOME/.local/bin/circom" + chmod +x "$HOME/.local/bin/circom" + echo "$HOME/.local/bin" >> "$GITHUB_PATH"🤖 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 @.github/workflows/circuits-ci.yml around lines 30 - 31, The workflow writes circom to /usr/local/bin which can fail due to permissions; change the install to a user-writable path (e.g., $HOME/.local/bin or $GITHUB_WORKSPACE/.local/bin) or prefix the curl/chmod commands with sudo. Update the two commands that reference /usr/local/bin/circom (the curl -o and chmod +x lines) to write to and set executable on the user-writable path (and ensure that path is added to PATH in the workflow) or use sudo before both commands if you intentionally want to write to /usr/local/bin.
🤖 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 @.github/workflows/circuits-ci.yml:
- Line 30: Replace the mutable download of circom by pinning to a specific
release and verifying its integrity: change the curl invocation that fetches
"circom-linux-amd64" (the line using curl -L ... -o /usr/local/bin/circom) to
download from a fixed tag URL (e.g.
/releases/download/vX.Y.Z/circom-linux-amd64), add a step to download the
corresponding published checksum (or vendor-provided SHA256/GPG signature),
verify the binary with sha256sum --check (or gpg --verify) before moving it into
/usr/local/bin, and use curl --fail and chmod +x only after verification to
ensure reproducible and secure installation.
In `@circuits/setup.mjs`:
- Around line 14-15: The contribution call using powersOfTau.contribute
currently constructs entropy with the predictable string 'markpool-entropy-' +
Date.now(); replace this with cryptographically secure randomness (e.g.,
generate random bytes via Node's crypto.randomBytes or Web Crypto
getRandomValues and encode as hex/base64) and use that value in both
powersOfTau.contribute calls so the entropy argument is unpredictable; update
the code paths that create the 'markpool-entropy-...' value to use the secure
random generator and propagate the encoded result into the contribute() calls.
In `@contracts/KNOWN_ISSUES.md`:
- Line 85: The current text calls MARKPoolVerifier "the settlement-specific
verifier contract", which is misleading; update the wording in KNOWN_ISSUES.md
so it states that MARKPoolVerifier is the shared pool verifier used for both
pool operations and settlement verification (e.g., replace "settlement-specific
verifier contract (MARKPoolVerifier)" with "shared pool verifier contract
(MARKPoolVerifier) used for both pool operations and settlement verification"),
and ensure the surrounding sentence that introduces MARKPoolVerifier as part of
the pool system (the paragraph referencing MARKPoolVerifier) reflects this
shared usage.
---
Duplicate comments:
In @.github/workflows/circuits-ci.yml:
- Around line 30-31: The workflow writes circom to /usr/local/bin which can fail
due to permissions; change the install to a user-writable path (e.g.,
$HOME/.local/bin or $GITHUB_WORKSPACE/.local/bin) or prefix the curl/chmod
commands with sudo. Update the two commands that reference /usr/local/bin/circom
(the curl -o and chmod +x lines) to write to and set executable on the
user-writable path (and ensure that path is added to PATH in the workflow) or
use sudo before both commands if you intentionally want to write to
/usr/local/bin.
In `@contracts/src/pool/MARKPool.sol`:
- Around line 72-73: The replay-protection mapping processedBridgeMessages is
only keyed by messageId which allows collisions across source chains; update the
logic to scope replay state by (srcChainId, messageId) instead: change the
mapping key usage in the bridgeIn processing and any reads/writes (references:
processedBridgeMessages and any place setting/checking it, e.g., inside
bridgeIn) to use a composite key like keccak256(abi.encodePacked(srcChainId,
messageId)), and do the same for nullifierWithdrawBinding if it is tied to
bridge messages; ensure all checks and assignments that previously used
messageId alone now use the composite key so different srcChainIds cannot
conflict.
In `@contracts/test/unit/pool/MARKPool.t.sol`:
- Around line 294-297: Replace the broad vm.expectRevert() in
testBridgeInRevertsWhenCallerNotRestricted with an assertion for the specific
AccessManaged unauthorized error: call
vm.expectRevert(abi.encodeWithSelector(AccessManaged.Unauthorized.selector))
before invoking pool.bridgeIn(...) so the test only passes for the intended
unauthorized revert coming from the AccessManaged contract.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: a78a9e44-734a-4f37-bb6c-63973aefe5ab
📒 Files selected for processing (8)
.github/workflows/circuits-ci.ymlcircuits/package.jsoncircuits/setup.mjscircuits/test/MARKPool.test.mjscontracts/KNOWN_ISSUES.mdcontracts/src/pool/MARKPool.solcontracts/src/pool/errors/PoolErrors.solcontracts/test/unit/pool/MARKPool.t.sol
- circuits-ci.yml: fix circom install permissions (use sudo mv to /usr/local/bin instead of direct write which fails on GH Actions) - PoolErrors.sol: add clarifying comment to FixedFeePolicy explaining it fires when minFee > 1 (not a fee-rate policy, a range guard) - MARKWithdrawAdapter.sol: document personal_sign intent on computeWithdrawIntentDigest (EIP-191 is intentional, not EIP-712) bridgeIn replay protection finding: already fixed in current code (processedBridgeMessages mapping + check at line 390) — stale finding.
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
contracts/KNOWN_ISSUES.md (1)
84-84:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClarify verifier ownership wording at Line 84.
MARKPoolVerifieris described as part of the pool system (Line 83), so calling it “settlement-specific” is misleading. Reword as a shared pool verifier wired into settlement.Proposed wording
-- **Settlement system** (`MARKSettlementModule` + `Groth16SettlementVerifier`): expects the same 13-signal layout via `IGroth16Verifier`. The settlement-specific verifier contract (`MARKPoolVerifier`) needs to be wired into `Groth16SettlementVerifier.setVerifierContract()` before ZK-based settlement is active. `AttestedSettlementVerifier` is the production-safe fallback until that wiring is complete. +- **Settlement system** (`MARKSettlementModule` + `Groth16SettlementVerifier`): expects the same 13-signal layout via `IGroth16Verifier`. The shared pool verifier contract (`MARKPoolVerifier`) must be wired into `Groth16SettlementVerifier.setVerifierContract()` before ZK-based settlement is active. `AttestedSettlementVerifier` is the production-safe fallback until that wiring is complete.🤖 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 `@contracts/KNOWN_ISSUES.md` at line 84, Update the wording to clarify that MARKPoolVerifier is a shared pool verifier (not "settlement-specific") that must be wired into the settlement flow; specifically, replace the phrase "The settlement-specific verifier contract (MARKPoolVerifier)" with wording that MARKPoolVerifier is the pool's shared verifier and must be set on Groth16SettlementVerifier via setVerifierContract() so the MARKSettlementModule (which uses IGroth16Verifier's 13-signal layout) can perform ZK-based settlement, with AttestedSettlementVerifier remaining the fallback until wiring is complete.circuits/setup.mjs (1)
14-15:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUse cryptographically secure entropy for setup contributions.
Lines 15 and 25 derive entropy from
Date.now(), which is predictable and weakens trusted-setup guarantees.Suggested fix
+import { randomBytes } from 'crypto'; import { zKey, powersOfTau } from 'snarkjs'; import { writeFileSync } from 'fs'; @@ await powersOfTau.contribute('build/pot15_0000.ptau', 'build/pot15_final.ptau', - 'MARK Protocol', 'markpool-entropy-' + Date.now()); + 'MARK Protocol', `markpool-entropy-${randomBytes(32).toString('hex')}`); @@ await zKey.contribute('build/markpool_0000.zkey', 'build/markpool_final.zkey', - 'MARK Protocol MARKPool', 'markpool-zkey-entropy-' + Date.now()); + 'MARK Protocol MARKPool', `markpool-zkey-entropy-${randomBytes(32).toString('hex')}`);Also applies to: 24-25
🤖 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 `@circuits/setup.mjs` around lines 14 - 15, The contribution call to powersOfTau.contribute currently builds entropy using the predictable Date.now() string ('markpool-entropy-' + Date.now()), which weakens the trusted setup; replace that construction with cryptographically secure randomness (e.g., generate a secure random buffer/hex string via Node's crypto.randomBytes or Web Crypto getRandomValues) and pass that entropy string to powersOfTau.contribute (update all occurrences that use 'markpool-entropy-' + Date.now()), ensuring the entropy is unpredictable and properly encoded before calling the contribute method..github/workflows/circuits-ci.yml (2)
23-27:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse an Active LTS Node runtime in CI.
Line 26 still pins Node
20; this is past EOL and should be upgraded to the current Active LTS line.Suggested fix
- name: Setup Node.js uses: actions/setup-node@v6 with: - node-version: '20' + node-version: '24'🤖 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 @.github/workflows/circuits-ci.yml around lines 23 - 27, The CI workflow step using actions/setup-node@v6 currently pins node-version: '20' which is past EOL; update the node-version value under the "Setup Node.js" step (actions/setup-node@v6) to an active LTS (either a specific supported major like '22' or use the dynamic alias 'lts/*') so the runner uses a current, supported Node runtime.
28-32:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin and verify the
circombinary instead of downloadinglatest.Line 30 pulls a mutable artifact (
releases/latest) with no checksum validation, which is a CI supply-chain risk.Suggested fix
- name: Install circom run: | - curl -L https://github.com/iden3/circom/releases/latest/download/circom-linux-amd64 -o circom + CIRCOM_VERSION="v2.1.9" + CIRCOM_URL="https://github.com/iden3/circom/releases/download/${CIRCOM_VERSION}/circom-linux-amd64" + CIRCOM_SHA256="<official_sha256>" + curl -fsSL "$CIRCOM_URL" -o circom + echo "${CIRCOM_SHA256} circom" | sha256sum -c - chmod +x circom sudo mv circom /usr/local/bin/circom🤖 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 @.github/workflows/circuits-ci.yml around lines 28 - 32, The CI step named "Install circom" currently downloads the mutable releases/latest binary via curl with no verification; change it to download a pinned release (use the specific tag URL like .../download/vX.Y.Z/circom) and verify the binary checksum before installing: add a step to fetch or embed the expected SHA256 (from a maintained source or CI secret/variable), compute the downloaded file's checksum (sha256sum) and fail if it doesn't match, then chmod +x and sudo mv /usr/local/bin/circom only after successful verification so the curl command in the "Install circom" step is both version-pinned and integrity-checked.contracts/test/unit/pool/MARKPool.t.sol (1)
294-297:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the exact revert for unauthorized
bridgeIn.A bare
vm.expectRevert()will pass on any failure, so this test won’t catch regressions wherebridgeInreverts for the wrong reason. Use the concreteAccessManaged/AccessManagerselector instead.In OpenZeppelin Contracts v5, what custom error is reverted when an unauthorized caller invokes an `AccessManaged` function guarded by `restricted`?🤖 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 `@contracts/test/unit/pool/MARKPool.t.sol` around lines 294 - 297, Replace the bare vm.expectRevert() in testBridgeInRevertsWhenCallerNotRestricted with a concrete expectRevert that matches the AccessManaged unauthorized custom error; specifically use vm.expectRevert(abi.encodeWithSelector(AccessManaged.Unauthorized.selector)) (or AccessManager.Unauthorized.selector if your contract imports that type) before calling pool.bridgeIn(...) so the test asserts the exact "Unauthorized" revert from AccessManaged rather than any revert.contracts/src/withdraw/MARKWithdrawAdapter.sol (1)
152-152:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
withdrawWithSigdebits from the wrong caller context.Line 152 calls
ASSET_LEDGER.debit(creditOwner, amount)from the adapter, but this PR’sRYLACreditLedger.debitis described asonlyPool. With the production ledger wired in, withdrawals will revert here becausemsg.senderis the adapter, notMARKPool. Route the burn through the pool or explicitly authorize the adapter in the ledger flow.🤖 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 `@contracts/src/withdraw/MARKWithdrawAdapter.sol` at line 152, withdrawWithSig currently calls ASSET_LEDGER.debit(creditOwner, amount) directly which fails because RYLACreditLedger.debit is onlyPool and expects MARKPool as msg.sender; instead route the burn through the pool or authorize the adapter. Update withdrawWithSig to call the MARKPool entrypoint that performs the ledger debit (e.g., MARKPool.withdrawCredit / MARKPool.burnCredit / whatever pool method exists that then calls ASSET_LEDGER.debit) so the ledger sees the pool as caller, or alternatively add explicit adapter authorization in RYLACreditLedger (modify RYLACreditLedger.debit’s access control to accept this adapter) and remove the direct ASSET_LEDGER.debit call. Ensure you reference withdrawWithSig, ASSET_LEDGER.debit, RYLACreditLedger.debit and MARKPool in your change.
🤖 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 `@circuits/setup.mjs`:
- Around line 32-33: The dynamic import with an import assertion for the EJS
template (const { default: solidityTemplate } = await import(...)) is
incompatible with Node 20+ and .ejs text imports; replace it by reading the
template as text (e.g., use readFile from 'node:fs/promises' to read
'node_modules/snarkjs/templates/verifier_groth16.sol.ejs' with 'utf8' and assign
the resulting string to solidityTemplate) and then call
zKey.exportSolidityVerifier('build/markpool_final.zkey', { groth16:
solidityTemplate }); alternatively, if you prefer, use createRequire from
'module' to resolve the module path and then read the file with fs; ensure
solidityTemplate is a string passed into zKey.exportSolidityVerifier.
- Around line 11-12: The script calls powersOfTau.newAccumulator('bn128', 15,
'build/pot15_0000.ptau') but never creates the build directory so the write can
ENOENT; before that call ensure the output directory exists (e.g., call
fs.promises.mkdir('build', { recursive: true }) or fs.mkdirSync('build', {
recursive: true })) and handle/await the promise so the directory is created
prior to invoking newAccumulator; add the mkdir call just before the
powersOfTau.newAccumulator invocation to guarantee the path for
'build/pot15_0000.ptau' exists.
In `@contracts/src/pool/MARKPool.sol`:
- Around line 29-32: Update the NatSpec in MARKPool.sol to correctly reflect the
signature scheme: change the line that claims
MARKWithdrawAdapter.withdrawWithSig() verifies EIP-712 signatures to state it
verifies EIP-191 / personal_sign signatures (via
MessageHashUtils.toEthSignedMessageHash). Mention
MARKWithdrawAdapter.withdrawWithSig() and
MessageHashUtils.toEthSignedMessageHash by name so integrators know the contract
expects personal_sign/EIP-191-style signatures rather than EIP-712 typed-data
signatures.
In `@contracts/test/unit/pool/MARKPool.t.sol`:
- Around line 292-305: Add a regression test that proves replay-protection on
messageId by invoking pool.bridgeIn twice with the same messageId and expecting
the second call to revert with BridgeMessageAlreadyProcessed; implement a new
test (e.g., testBridgeInRevertsOnReplayOfMessageId) that sets up commitments =
[C0, C1], uses vm.prank(admin) to call pool.bridgeIn(targetChainId,
bytes32(uint256(1)), commitments) once successfully and then calls it again with
the same messageId and wraps the second call with
vm.expectRevert(BridgeMessageAlreadyProcessed.selector) to assert the replay is
rejected.
---
Duplicate comments:
In @.github/workflows/circuits-ci.yml:
- Around line 23-27: The CI workflow step using actions/setup-node@v6 currently
pins node-version: '20' which is past EOL; update the node-version value under
the "Setup Node.js" step (actions/setup-node@v6) to an active LTS (either a
specific supported major like '22' or use the dynamic alias 'lts/*') so the
runner uses a current, supported Node runtime.
- Around line 28-32: The CI step named "Install circom" currently downloads the
mutable releases/latest binary via curl with no verification; change it to
download a pinned release (use the specific tag URL like
.../download/vX.Y.Z/circom) and verify the binary checksum before installing:
add a step to fetch or embed the expected SHA256 (from a maintained source or CI
secret/variable), compute the downloaded file's checksum (sha256sum) and fail if
it doesn't match, then chmod +x and sudo mv /usr/local/bin/circom only after
successful verification so the curl command in the "Install circom" step is both
version-pinned and integrity-checked.
In `@circuits/setup.mjs`:
- Around line 14-15: The contribution call to powersOfTau.contribute currently
builds entropy using the predictable Date.now() string ('markpool-entropy-' +
Date.now()), which weakens the trusted setup; replace that construction with
cryptographically secure randomness (e.g., generate a secure random buffer/hex
string via Node's crypto.randomBytes or Web Crypto getRandomValues) and pass
that entropy string to powersOfTau.contribute (update all occurrences that use
'markpool-entropy-' + Date.now()), ensuring the entropy is unpredictable and
properly encoded before calling the contribute method.
In `@contracts/KNOWN_ISSUES.md`:
- Line 84: Update the wording to clarify that MARKPoolVerifier is a shared pool
verifier (not "settlement-specific") that must be wired into the settlement
flow; specifically, replace the phrase "The settlement-specific verifier
contract (MARKPoolVerifier)" with wording that MARKPoolVerifier is the pool's
shared verifier and must be set on Groth16SettlementVerifier via
setVerifierContract() so the MARKSettlementModule (which uses IGroth16Verifier's
13-signal layout) can perform ZK-based settlement, with
AttestedSettlementVerifier remaining the fallback until wiring is complete.
In `@contracts/src/withdraw/MARKWithdrawAdapter.sol`:
- Line 152: withdrawWithSig currently calls ASSET_LEDGER.debit(creditOwner,
amount) directly which fails because RYLACreditLedger.debit is onlyPool and
expects MARKPool as msg.sender; instead route the burn through the pool or
authorize the adapter. Update withdrawWithSig to call the MARKPool entrypoint
that performs the ledger debit (e.g., MARKPool.withdrawCredit /
MARKPool.burnCredit / whatever pool method exists that then calls
ASSET_LEDGER.debit) so the ledger sees the pool as caller, or alternatively add
explicit adapter authorization in RYLACreditLedger (modify
RYLACreditLedger.debit’s access control to accept this adapter) and remove the
direct ASSET_LEDGER.debit call. Ensure you reference withdrawWithSig,
ASSET_LEDGER.debit, RYLACreditLedger.debit and MARKPool in your change.
In `@contracts/test/unit/pool/MARKPool.t.sol`:
- Around line 294-297: Replace the bare vm.expectRevert() in
testBridgeInRevertsWhenCallerNotRestricted with a concrete expectRevert that
matches the AccessManaged unauthorized custom error; specifically use
vm.expectRevert(abi.encodeWithSelector(AccessManaged.Unauthorized.selector)) (or
AccessManager.Unauthorized.selector if your contract imports that type) before
calling pool.bridgeIn(...) so the test asserts the exact "Unauthorized" revert
from AccessManaged rather than any revert.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: a8f51f1a-f215-4ac2-865d-6e829f502b3f
📒 Files selected for processing (9)
.github/workflows/circuits-ci.ymlcircuits/package.jsoncircuits/setup.mjscircuits/test/MARKPool.test.mjscontracts/KNOWN_ISSUES.mdcontracts/src/pool/MARKPool.solcontracts/src/pool/errors/PoolErrors.solcontracts/src/withdraw/MARKWithdrawAdapter.solcontracts/test/unit/pool/MARKPool.t.sol
- setup.mjs: use crypto.randomBytes for ceremony entropy (Date.now is predictable), add mkdirSync for build/, fix EJS template loading to use readFileSync instead of dynamic import with assert (unsupported in Node 20/22/24 ESM) - circuits-ci.yml: pin circom to v2.2.3 instead of latest, add version verification step - KNOWN_ISSUES.md: fix misleading 'settlement-specific verifier' wording — MARKPoolVerifier is a shared pool verifier, not settlement-specific - MARKPool.sol: fix NatSpec EIP-712 reference to EIP-191 (personal_sign)
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.github/workflows/circuits-ci.yml:
- Around line 11-19: The workflow job "circuits-test" lacks an explicit
permissions block; add a top-level permissions mapping under the circuits-test
job that declares only the least-privilege scopes this job needs (e.g., set
contents: read and any other minimal tokens the job actually uses such as
id-token: write or checks: read only if required), placing the permissions block
directly under the job definition so the job no longer inherits broader repo/org
defaults.
In `@circuits/setup.mjs`:
- Around line 38-39: The code uses URL.pathname which can produce invalid
Windows paths and won't decode percent-encoding; instead convert the file URL to
a proper file system path using fileURLToPath before reading. Replace usage of
new URL(...).pathname when computing templatePath with fileURLToPath(new
URL('node_modules/snarkjs/templates/verifier_groth16.sol.ejs', import.meta.url))
and then pass that result to readFileSync to set solidityTemplate so
readFileSync receives a correct platform-safe path.
In `@contracts/src/pool/MARKPool.sol`:
- Line 389: The check ensuring messageId != bytes32(0) currently reverts with
InvalidRoot(), which is semantically wrong; add a new error InvalidMessageId()
to PoolErrors (or the contract's errors list) and replace the revert on
messageId validation in MARKPool (the line using messageId == bytes32(0)) to
revert InvalidMessageId() so the error accurately reflects the failing
condition.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 6005412f-9ccd-41bc-bda7-2bad36b81d91
📒 Files selected for processing (4)
.github/workflows/circuits-ci.ymlcircuits/setup.mjscontracts/KNOWN_ISSUES.mdcontracts/src/pool/MARKPool.sol
RYLACreditLedger:
- Separate credit (pool-only) and debit (adapter-only) callers
- Add setAdapter() one-time setter to break circular deploy dependency
(adapter constructor needs ledger, ledger needs adapter address)
- Add AdapterAlreadySet error
DeployMARKPool.s.sol:
- Call ledger.setAdapter(adapter) after adapter deployment
Tests:
- RYLACreditLedger.t.sol: updated for new caller model, 11 tests
- MARKWithdrawAdapter.t.sol: add setAdapter call in setUp
- MARKPoolE2E.t.sol: full withdrawal flow E2E test (3 tests)
- testFullWithdrawalFlow: mint RYLA -> transactWithWithdrawBinding
-> withdrawWithSig -> verify RYLA burned, ETH received
- testNullifierReplayRejected
- testBindingMismatchRejected
134/134 tests pass
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@circuits/test/MARKPool.test.mjs`:
- Around line 52-54: The failure-matching uses the raw error text in variable
msg (from e?.message) which makes matches case-sensitive; normalize msg to
lowercase and perform includes checks against lowercase patterns so that the
conditional in the expect-fail path (the if that logs `PASS: ${label}`) uses
msg.toLowerCase() and compares to 'assert failed', 'constraint', and 'error in
template' to avoid flaky negatives.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: cc3f2807-5eb3-43df-9ad1-3862c320d939
📒 Files selected for processing (3)
circuits/test/MARKPool.test.mjscontracts/KNOWN_ISSUES.mdcontracts/src/pool/MARKPool.sol
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Documents the two post-deploy calls required to activate ZK-based settlement: setSettlementModule and setVerifierContract on Groth16SettlementVerifier, then setVerifier on MARKSettlementModule. AttestedSettlementVerifier remains the fallback until wiring is complete.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
contracts/KNOWN_ISSUES.md (1)
77-77: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winKI-7 title emphasizes separation over the critical shared-circuit architecture.
For audit-facing documentation, the key security-relevant fact is that both domains share a single circuit implementation. The current title leads with "Two separate ZK systems," which emphasizes architectural division rather than the shared proving circuit. Auditors reviewing circuit security need to immediately understand they're evaluating one circuit used in two contexts, not two independent ZK implementations.
📝 Suggested title that leads with the shared circuit
-## KI-7: Two separate ZK systems sharing the MARKPool 13-signal circuit +## KI-7: Pool and settlement domains share the MARKPool 13-signal circuit🤖 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 `@contracts/KNOWN_ISSUES.md` at line 77, The title for KI-7 currently emphasizes separate ZK systems; update the heading so it leads with the shared circuit to make the security impact explicit—for example change the "## KI-7: Two separate ZK systems sharing the MARKPool 13-signal circuit" heading to something like "## KI-7: Single MARKPool 13-signal circuit used by two ZK systems" (referencing KI-7 and MARKPool 13-signal circuit) so auditors immediately see one circuit is shared across contexts.
🤖 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 `@contracts/KNOWN_ISSUES.md`:
- Line 77: The title for KI-7 currently emphasizes separate ZK systems; update
the heading so it leads with the shared circuit to make the security impact
explicit—for example change the "## KI-7: Two separate ZK systems sharing the
MARKPool 13-signal circuit" heading to something like "## KI-7: Single MARKPool
13-signal circuit used by two ZK systems" (referencing KI-7 and MARKPool
13-signal circuit) so auditors immediately see one circuit is shared across
contexts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 03d45cbd-1852-483d-888c-588699c560df
📒 Files selected for processing (3)
circuits/test/MARKPool.test.mjscontracts/KNOWN_ISSUES.mdcontracts/src/pool/MARKPool.sol
iap
dismissed stale reviews from coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], coderabbitai[bot], and coderabbitai[bot]
All findings addressed in subsequent commits.
iap
added a commit
that referenced
this pull request
May 17, 2026
* chore(deps): bump actions/setup-node from 5 to 6
Bumps [actions/setup-node](https://github.com/actions/setup-node) from 5 to 6.
- [Release notes](https://github.com/actions/setup-node/releases)
- [Commits](https://github.com/actions/setup-node/compare/v5...v6)
---
updated-dependencies:
- dependency-name: actions/setup-node
dependency-version: '6'
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
* chore(deps): bump actions/upload-artifact from 4 to 7
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 7.
- [Release notes](https://github.com/actions/upload-artifact/releases)
- [Commits](https://github.com/actions/upload-artifact/compare/v4...v7)
---
updated-dependencies:
- dependency-name: actions/upload-artifact
dependency-version: '7'
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
* chore(deps): bump actions/checkout from 5 to 6
Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](https://github.com/actions/checkout/compare/v5...v6)
---
updated-dependencies:
- dependency-name: actions/checkout
dependency-version: '6'
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
* chore(deps): bump actions/github-script from 7 to 9
Bumps [actions/github-script](https://github.com/actions/github-script) from 7 to 9.
- [Release notes](https://github.com/actions/github-script/releases)
- [Commits](https://github.com/actions/github-script/compare/v7...v9)
---
updated-dependencies:
- dependency-name: actions/github-script
dependency-version: '9'
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
* chore(deps): bump the frontend-minor-patch group with 13 updates
Bumps the frontend-minor-patch group with 13 updates:
| Package | From | To |
| --- | --- | --- |
| [@eth-optimism/viem](https://github.com/ethereum-optimism/ecosystem/tree/HEAD/packages/viem) | `0.3.2` | `0.4.15` |
| [@radix-ui/react-separator](https://github.com/radix-ui/primitives) | `1.1.2` | `1.1.8` |
| [@radix-ui/react-slot](https://github.com/radix-ui/primitives) | `1.1.2` | `1.2.4` |
| [@tailwindcss/vite](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/@tailwindcss-vite) | `4.0.6` | `4.2.4` |
| [@tanstack/react-query](https://github.com/TanStack/query/tree/HEAD/packages/react-query) | `5.66.0` | `5.100.8` |
| [abitype](https://github.com/wevm/abitype) | `1.0.8` | `1.2.4` |
| [tailwind-merge](https://github.com/dcastil/tailwind-merge) | `3.0.1` | `3.5.0` |
| [tailwindcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/tailwindcss) | `4.0.6` | `4.2.4` |
| [viem](https://github.com/wevm/viem) | `2.23.1` | `2.48.8` |
| [eslint-plugin-react-refresh](https://github.com/ArnaudBarre/eslint-plugin-react-refresh) | `0.4.19` | `0.5.2` |
| [mprocs](https://github.com/pvolok/mprocs) | `0.7.2` | `0.9.2` |
| [prettier](https://github.com/prettier/prettier) | `3.5.0` | `3.8.3` |
| [typescript-eslint](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint) | `8.24.0` | `8.59.1` |
Updates `@eth-optimism/viem` from 0.3.2 to 0.4.15
- [Changelog](https://github.com/ethereum-optimism/ecosystem/blob/main/packages/viem/CHANGELOG.md)
- [Commits](https://github.com/ethereum-optimism/ecosystem/commits/HEAD/packages/viem)
Updates `@radix-ui/react-separator` from 1.1.2 to 1.1.8
- [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md)
- [Commits](https://github.com/radix-ui/primitives/commits)
Updates `@radix-ui/react-slot` from 1.1.2 to 1.2.4
- [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md)
- [Commits](https://github.com/radix-ui/primitives/commits)
Updates `@tailwindcss/vite` from 4.0.6 to 4.2.4
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.2.4/packages/@tailwindcss-vite)
Updates `@tanstack/react-query` from 5.66.0 to 5.100.8
- [Release notes](https://github.com/TanStack/query/releases)
- [Changelog](https://github.com/TanStack/query/blob/main/packages/react-query/CHANGELOG.md)
- [Commits](https://github.com/TanStack/query/commits/@tanstack/react-query@5.100.8/packages/react-query)
Updates `abitype` from 1.0.8 to 1.2.4
- [Release notes](https://github.com/wevm/abitype/releases)
- [Commits](https://github.com/wevm/abitype/compare/abitype@1.0.8...abitype@1.2.4)
Updates `tailwind-merge` from 3.0.1 to 3.5.0
- [Release notes](https://github.com/dcastil/tailwind-merge/releases)
- [Commits](https://github.com/dcastil/tailwind-merge/compare/v3.0.1...v3.5.0)
Updates `tailwindcss` from 4.0.6 to 4.2.4
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.2.4/packages/tailwindcss)
Updates `viem` from 2.23.1 to 2.48.8
- [Release notes](https://github.com/wevm/viem/releases)
- [Commits](https://github.com/wevm/viem/compare/viem@2.23.1...viem@2.48.8)
Updates `eslint-plugin-react-refresh` from 0.4.19 to 0.5.2
- [Release notes](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/releases)
- [Changelog](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/blob/main/CHANGELOG.md)
- [Commits](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/compare/v0.4.19...v0.5.2)
Updates `mprocs` from 0.7.2 to 0.9.2
- [Release notes](https://github.com/pvolok/mprocs/releases)
- [Changelog](https://github.com/pvolok/mprocs/blob/master/CHANGELOG.md)
- [Commits](https://github.com/pvolok/mprocs/compare/v0.7.2...v0.9.2)
Updates `prettier` from 3.5.0 to 3.8.3
- [Release notes](https://github.com/prettier/prettier/releases)
- [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md)
- [Commits](https://github.com/prettier/prettier/compare/3.5.0...3.8.3)
Updates `typescript-eslint` from 8.24.0 to 8.59.1
- [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases)
- [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/CHANGELOG.md)
- [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.59.1/packages/typescript-eslint)
---
updated-dependencies:
- dependency-name: "@eth-optimism/viem"
dependency-version: 0.4.15
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@radix-ui/react-separator"
dependency-version: 1.1.8
dependency-type: direct:production
update-type: version-update:semver-patch
dependency-group: frontend-minor-patch
- dependency-name: "@radix-ui/react-slot"
dependency-version: 1.2.4
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/vite"
dependency-version: 4.2.4
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tanstack/react-query"
dependency-version: 5.100.8
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: abitype
dependency-version: 1.2.4
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: tailwind-merge
dependency-version: 3.5.0
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: tailwindcss
dependency-version: 4.2.4
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: viem
dependency-version: 2.48.8
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: eslint-plugin-react-refresh
dependency-version: 0.5.2
dependency-type: direct:development
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: mprocs
dependency-version: 0.9.2
dependency-type: direct:development
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: prettier
dependency-version: 3.8.3
dependency-type: direct:development
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: typescript-eslint
dependency-version: 8.59.1
dependency-type: direct:development
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
...
Signed-off-by: dependabot[bot] <support@github.com>
* fix(readiness): run pre-checks before contracts working directory exists
* fix(frontend): remove non-component export from button ui
* ci(security): add codeql and dependency review gates
* chore(security): add local slither install and core scan targets
* docs(phase1): add comprehensive contributor & deployment runbooks
Add Phase 1 foundation documentation for team scaling and professional maintenance:
CONTRIBUTING.md:
- Local development setup instructions (Node, Foundry, super-cli)
- Feature branch workflow with conventional commits
- Code standards (TypeScript, Solidity, Testing)
- PR submission checklist and review process
- Testing guidelines and test structure
- Troubleshooting for common dev issues
DEPLOYMENT.md:
- Step-by-step staging deployment runbook (OP Sepolia)
- Mainnet deployment procedures with gates
- Pre/post-deployment checklists
- Evidence generation and verification
- Monitoring and health checks
- Rollback procedures for emergency scenarios
- Comprehensive troubleshooting guide
- Command cheat sheet and timeline estimates
TROUBLESHOOTING.md:
- Development setup issues (pnpm, Node, Foundry, super-cli, git hooks)
- Smart contract issues (architecture guard, layering guard, Slither findings)
- Frontend development issues (port conflicts, TypeScript errors, module resolution)
- Testing issues (hanging tests, gas, balance)
- Deployment issues (insufficient funds, timeouts, RPC problems)
- CI/CD workflow issues (stuck workflows, secrets, version mismatches)
- Network & RPC issues (timeouts, contract not found, chain ID)
.github/CODEOWNERS:
- Enhanced documentation with clear sections
- Added review requirements annotations
- Better organization for team scaling
- Maintains strict single-owner model (ready for multi-owner when scaling)
Impact:
- Enables solo maintainer to self-document workflows
- Provides clear onboarding path for new contributors
- Establishes professional deployment procedures
- Reduces support burden with comprehensive troubleshooting
- Foundation for team collaboration (docs ready for team addition)
- Production-ready documentation for auditors and stakeholders
This commit fulfills Phase 1 foundation requirements:
✅ CONTRIBUTING.md created
✅ DEPLOYMENT.md runbook created
✅ TROUBLESHOOTING.md created
✅ CODEOWNERS enhanced and documented
Ready for: Phase 2 (interactive UI) and Phase 3 (security audit planning)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* chore(deps): bump github/codeql-action from 3 to 4 (#16)
Bump github/codeql-action from v3 to v4 to resolve Node.js 20 deprecation warnings on CI.
* chore(ci): bump dependency-review-action from v4 to v5
* chore(ci): disable CodeQL triggers until repo transferred to org with GHAS
* Enable org-transfer governance: CodeQL, Gitleaks, release-gate container, and verification scripts (#19)
* docs: replace roadmap with lean security next-steps guide
* fix(docs): remove duplicate required-check entries in BRANCHING.md
* fix(ci): add USER root in release-gate Dockerfile for apt-get permissions
* ci(security): fix dependency review tag and use OSS gitleaks CLI
* ci(security): fix gitleaks PATH on github runner
* ci(security): run gitleaks scan via docker image
* ci(security): remove hardcoded key and scope gitleaks to workspace
* ci(contracts): fix anvil key extraction for release check
* ci(contracts): require 64-byte anvil private key extraction
* ci: always run contracts/frontend checks on protected branches (#21)
* ci: phase-1 reusable workflows for frontend, slither, and secrets scan (#23)
* ci: extract reusable frontend/slither/secrets workflows
* ci(security): apply codereview pinning and permissions fixes
* fix(contracts): bridge approval safety + IRYLA interface decoupling
- Wrap sendERC20 in try/catch; clear approval and revert with BridgeFailed() on failure
- Extract IRYLA interface (inherits IERC20); MARKSettlementModule decoupled from concrete RYLA type
- Add unit test for BridgeFailed catch branch
* docs: sync governance and CI docs with current protections
- Add missing required checks (Secrets Drift Guard, Release Gate Container) to all branch matrices
- Fix Analyze (JavaScript/TypeScript) casing to match canonical check names
- Fixes Validate Governance Policy Consistency CI check
* chore(deps): bump frontend minor/patch dependencies
105 minor and patch updates including:
- @tanstack/react-query 5.100.8 → 5.100.9
- typescript-eslint 8.59.1 → 8.59.2
- bufferutil 4.0.9 → 4.1.0
- jiti 2.6.1 → 2.7.0
- lockfile resolutions updated accordingly
All CI checks pass on Node 20 and 22.
* fix(deps): bump vite 6.1.0 → 6.4.2 (security)
Fixes high-severity arbitrary file read CVE and medium-severity path traversal in vite dev server.
* test(contracts): add missing unit test coverage
71 tests (was 59). Covers zero-input guards, exact error selectors, accumulator resets, supportsInterface, and isMint flag binding.
* chore(governance): migrate CODEOWNERS to @trade/maintainers team
Replaces @iap with @trade/maintainers across all CODEOWNERS entries. Team created with maintain permission on repo.
* chore(ci): switch CodeRabbit to assertive profile
profile: chill → assertive, request_changes_workflow: false → true
* fix(docs): add VALIDATE_MODE to staging checklist prerequisites
Adds missing VALIDATE_MODE env var to staging checklist. Clarifies operator/attester rotation step with RUNBOOK.md reference. Removes trailing newline from package.json.
* chore(docs): remove stale pre-transfer planning documents
Removes TRANSFER_NOW_CHECKLIST.md, ORG_TRANSFER_SECURITY_CHECKLIST.md, SECURITY_NEXT_STEPS.md, PROJECT_REVIEW.md — all completed with the org transfer on May 6, 2026.
* chore(governance): clean up CODEOWNERS
Remove decorative section dividers, redundant comments, and duplicate entry. Consolidate contract path globs.
* fix(ci): workflow correctness and consistency fixes
Pin slither-analyzer==0.11.5, fix secrets-drift-guard false positives, fix verify-governance.sh dismiss_stale_reviews on dev, add canary to evidence-manifest trigger, fix inputs context, fix wait-port, add pull_request_target comments, add Docker layer caching.
* feat(contracts): migrate AttestedSettlementVerifier to EIP-712
Replace hybrid EIP-191 pattern with standard EIP-712 typed data signing. Expose settlementDigest() for off-chain signers. Add NatSpec on proof encoding and contextHash. 71 tests pass.
* chore: improve gitignore coverage
Add .env/.env.*/*.env and supersim-logs/ to root gitignore. Add coverage/ to contracts gitignore.
* fix(ci): reliability and correctness fixes
Add timeout-minutes:15 to stuck jobs, replace rg with grep -Eo in smoke script, pin slither==0.11.5 in Makefile, add explicit invariant runs=256 to foundry.toml.
* chore(deps): ignore transitive alerts from super-cli
Ignore @hono/node-server, drizzle-orm, @stablelib/ed25519 scoped to vulnerable versions — all transitive from super-cli dev tool, no upstream fix available.
* docs: add SECURITY.md
Reporting channel, scope, response SLA, and supported versions.
* chore(deps): bump @types/node from 22.13.1 to 25.6.1
Type definitions update.
* chore(deps): bump typescript from 5.7.3 to 6.0.3
Add ignoreDeprecations:6.0 for baseUrl deprecation warning.
* chore(deps): bump frontend-minor-patch group
viem, debug, and other minor/patch updates.
* chore(deps): bump docker/setup-buildx-action from 3 to 4
Node 24 runtime update.
* chore(deps): bump frontend-minor-patch group
Minor/patch frontend dependency updates.
* fix: stale references and check name mismatches
Remove chainId double-encoding from AttestedSettlementVerifier, fix stale iap/mark URLs, fix governance script check names to match actual CI output.
* test(contracts): add bridge integration test against supersim
Exercises MARKBridgeAdapter against live SuperchainTokenBridge on two supersim forks. Verifies cross-chain token transfer and rate limit enforcement.
* test(contracts): add bridge adapter invariant fuzz tests
Three invariants covering rate limiting: daily cap never exceeded, accumulator consistent with cap, zero address never holds operator role. 74 tests pass.
* fix(governance): sync check lists and fix ruleset condition
Fix ruleset condition bug (canary/main now covered), sync apply-governance.sh and verify-governance.sh with live branch protection, fix frontend check name prefix in docs.
* chore(governance): document new ruleset structure
Two focused rulesets: branch-protection (CodeQL alert gate) and tag-protection (v* tags). Replaces the broken develop ruleset.
* feat(token): rename RYLA display name to 'RYLA Credits'
name() returns 'RYLA Credits', symbol stays 'RYLA'. Test and verification script updated.
* test
Documents key roles and trust assumptions, attester key rotation
procedure, break-glass procedure, production mode implications,
and key storage recommendations for auditors and operators.
* fix(ci): use matrix language as CodeQL job name
Produces consistent check name 'Analyze (javascript-typescript)' matching branch protection requirements.
* chore(config): harden staging profile and document environment setup
Remove PRIVATE_KEY from staging.env, fix bridge destination to OP Sepolia, add key separation docs, fix env guard and drift guard for CI validation.
* feat(frontend): replace dev dashboard with protocol info page
Protocol info page with pre-production status, contract descriptions, and resource links. Providers updated to optimism/optimismSepolia.
* chore(docs): cleanup and NatSpec improvements
Fix README clone URL and naming, remove stale date from CONTRIBUTING.md, add eip712Domain NatSpec and no-pause design decision docs.
* fix(contracts): document setVerifier interface check limitation
Add @dev comment explaining code.length check rejects EOAs but not non-conforming contracts.
* docs: add protocol philosophy to README
Code is a rule. No DAO, no drama. Don't Trust, Verify.
* fix(ci): add working-directory override to pre-checkout branch enforcement steps
Fixes pre-checkout branch check failing with 'No such file or directory' in staging and production workflows.
* fix(ops): enable post-deploy in rehearse-production-lock
Enable MARK_RELEASE_RUN_POSTDEPLOY so activateProductionMode() is called during rehearsal.
* fix(ops): export deployed verifier address to env before PostDeployMARKSetup
Fixes VerifierRequiredWhenProofEnabled during staging rehearsal.
* fix(ci): exclude Anvil default key from secrets drift guard
Syncs Anvil key exclusion to dev.
* test
THREAT_MODEL.md: trust boundaries, role compromise impact, external
dependencies, invariants, and explicit out-of-scope items.
KNOWN_ISSUES.md: six accepted design decisions with rationale —
attested verifier as ZK placeholder, no-pause design, setVerifier
interface check limitation, counter overflow analysis, timestamp
epoch manipulation, and transitive dep alerts.
* fix(docs): correct two inaccurate invariants in THREAT_MODEL.md
consumedIntents is set after proof validation, not before. Module balance invariant is per-operation, not absolute zero.
* fix(contracts): move consumedIntents assignment before external call (CEI)
Follows CEI pattern — marks intent consumed before external verifier call. No behaviour change for current view verifier.
* chore(governance): set canary to 0 required approvals for solo maintainer
Solo dev cannot self-approve. CI checks are the gate. Restore to 1 when second team member joins.
* docs(contracts): add NatSpec to settleMint and settleBurn
Documents pre-approval requirement for settleBurn.
* fix(ops): wait for tx confirmation in staging rehearsal
Add --slow to forge script broadcast so Foundry waits for each transaction receipt before the verify step runs.
* fix(governance): set all branches to 0 required approvals
Solo maintainer cannot approve own PRs. CI gates are the enforcement mechanism. Removes MAIN_REVIEW_COUNT/DEV_REVIEW_COUNT vars, adds approval count verification to verify-governance.sh.
* fix(governance): restrict direct pushes to trade/maintainers team
Restricts direct pushes on all branches to trade/maintainers team. Removes unused helper functions. verify-governance.sh now checks push restriction team slug.
* fix(deps): update drizzle-orm dependabot ignore rule to 0.38.4
drizzle-orm@0.38.4 is transitive from @eth-optimism/super-cli. Updated ignore rule to match installed version. All four Dependabot alerts dismissed as tolerable risk.
* feat(contracts): add Groth16SettlementVerifier
Adds Groth16SettlementVerifier implementing IUTXOSettlementVerifier via swappable IGroth16Verifier. 12 unit tests passing. AttestedSettlementVerifier remains active production verifier.
* feat(circuits): add UTXOSettlement circom circuit
Adds UTXOSettlement circom circuit. Poseidon-based UTXO ownership proof. 602 constraints, 6 witness tests passing.
* feat(contracts): add MARKPool ZK UTXO pool domain
Adds MARKPool shielded RYLA transfer pool. 88 unit tests passing.
* fix(contracts): rewrite MARKPool for MARK's 4-signal circuit
Rewrites MARKPool from scratch for MARK's own UTXOSettlement circuit. UTXOVerifier.sol regenerated from MARK's own trusted setup. 84 unit tests passing.
* fix(circuits): add range constraints and isMint burn path
Range constraints on recipient/chainId/settlementModule/amount. isMint burn path in MARKPool. Trusted setup rerun. 84 tests passing.
* feat(pool): add MARKPool ZK UTXO pool domain (#100)
* feat(pool): add MARKPool ZK UTXO pool domain
Introduces the full pool domain for private RYLA transfers:
Contracts:
- MARKPool: ZK UTXO pool with Merkle tree, fee policy, bridge-out/in,
withdraw binding, AccessManaged access control
- MARKWithdrawAdapter: EIP-712 signature-based withdrawal adapter
- RYLACreditLedger: ICreditLedger adapter bridging MARKPool to RYLA
mint/burn; restricted to pool caller only (onlyPool)
- PoolFeePolicy, PoolPublicInputs, PoolValidation: pool support libraries
- MARKPoolVerifier: Groth16 verifier generated from MARKPool circuit
(13 public signals, pot15 trusted setup)
Interfaces: ICreditLedger, IVerifier, IPoolBridge, IPoolNullifier
Crypto: MerkleTree (Poseidon, depth-20), ProofUtils, PoseidonT3
Circuit:
- circuits/mark/MARKPool.circom: MARK-native UTXO circuit (depth=20,
2-in/2-out, 13 public signals); renamed from prototype utxo.circom,
domain constants documented as permanent, hardcoded fee policy removed
- circuits/setup.mjs: trusted setup script (pot15)
- circuits/test/MARKPool.test.mjs: 13 witness tests
CI: circuits-ci.yml runs witness tests on every PR
Tests: MARKPool.t.sol (22), MARKWithdrawAdapter.t.sol (9),
RYLACreditLedger.t.sol (8)
* fix(pool): fix PoolErrors, domain separators, remove dead code
- PoolErrors.sol: rewrite to match Pool.sol, PoolValidation.sol, and
MerkleTree.sol — adds 25 missing errors (build was broken), removes
18 errors only used by the old MARKPool prototype
- MARKPool.sol: rename domain separator Pool.WithdrawBinding.v1 to
MARKPool.WithdrawBinding.v1 (permanent, must be set before deploy)
- MARKWithdrawAdapter.sol: rename domain separator
WithdrawAdapter.Intent.v1 to MARKWithdrawAdapter.Intent.v1
- UTXOVerifier.sol: delete (built for old 4-signal circuit, wrong
interface, superseded by MARKPoolVerifier.sol)
- IUTXOVerifier.sol: delete (superseded by IVerifier.sol)
- UTXOSettlement.circom: delete (superseded by MARKPool.circom)
- Groth16SettlementVerifier.sol: update stale comment
- KNOWN_ISSUES.md: add KI-7 (two-circuit architecture), KI-8 (pool
domain access control model)
- foundry.toml: via_ir = true for pool domain compilation
* fix(pool): immutable naming, deploy script, docs, invariants, arch guard
- MARKPool, MARKWithdrawAdapter: rename immutables to SCREAMING_SNAKE_CASE
(assetLedger->ASSET_LEDGER, proofPool->PROOF_POOL)
- MARKPool: remove _assetLedger from constructor; add setAssetLedger()
one-time restricted setter to break circular deploy dependency with
RYLACreditLedger
- DeployMARKPool.s.sol: full deployment script for pool domain
(AccessManager, MARKPool, RYLACreditLedger, MARKWithdrawAdapter)
- MARKPool.sol: add withdrawal flow NatSpec (burn-to-claim model)
- ARCHITECTURE.md: add pool/withdraw domains, dependency rules, and
withdrawal flow section
- MARKPoolInvariants.t.sol: 3 invariants (nullifiers never unspent,
withdraw bindings immutable, root queue only grows)
- architecture-guard.sh: add pool->settlement/bridge and
withdraw->settlement/bridge isolation rules
* fix(pool): fix deploy script role grant and ASSET_LEDGER null guard
- DeployMARKPool.s.sol: grant POOL_ADMIN_ROLE to deployer during setup
so setAssetLedger/setIntentSigner calls succeed when deployer != owner;
revoke deployer role after setup completes
- MARKPool._applyFee: revert InvalidAssetLedger if ASSET_LEDGER is not
set and a non-zero fee is applied (prevents silent call to address(0))
* fix(ci): compile circuit before running witness tests
circuits/build/ is gitignored so the WASM and witness_calculator.js
are not in the repo. Add circom install and npm run build steps before
npm test so CI compiles the circuit fresh on each run.
* fix(ci): create build dir before circom compile
* refactor(pool): pre-merge improvements
- Rename immutables to SCREAMING_SNAKE_CASE: assetLedger->ASSET_LEDGER,
proofPool->PROOF_POOL (MARKPool.sol, MARKWithdrawAdapter.sol)
- MARKPool: remove _assetLedger from constructor, add setAssetLedger()
one-time restricted setter to break circular deploy dependency with
RYLACreditLedger
- MARKPool: add withdrawal flow documentation to contract NatSpec
- ARCHITECTURE.md: add pool/withdraw domains, dependency rules, and
withdrawal flow explanation
- DeployMARKPool.s.sol: deployment script for MARKPool, RYLACreditLedger,
MARKWithdrawAdapter with AccessManager configuration
- MARKPoolInvariants.t.sol: 3 invariants (nullifiers never unspent,
withdraw bindings immutable, root queue only grows)
- architecture-guard.sh: add pool and withdraw domain isolation rules
* chore(pool): update circuits CI, setup, and pool errors
- circuits-ci.yml: updated to run MARKPool witness tests
- circuits/package.json: build/test scripts point to MARKPool.circom
- circuits/setup.mjs: updated for MARKPool.circom trusted setup
- circuits/test/MARKPool.test.mjs: cleaned up test file
- contracts/KNOWN_ISSUES.md: updated KI-7 for current two-circuit state
- contracts/src/pool/errors/PoolErrors.sol: add missing blank line
* fix(pool): address CodeRabbit review findings
- circuits-ci.yml: fix circom install permissions (use sudo mv to
/usr/local/bin instead of direct write which fails on GH Actions)
- PoolErrors.sol: add clarifying comment to FixedFeePolicy explaining
it fires when minFee > 1 (not a fee-rate policy, a range guard)
- MARKWithdrawAdapter.sol: document personal_sign intent on
computeWithdrawIntentDigest (EIP-191 is intentional, not EIP-712)
bridgeIn replay protection finding: already fixed in current code
(processedBridgeMessages mapping + check at line 390) — stale finding.
* fix(pool): address second round CodeRabbit findings
- setup.mjs: use crypto.randomBytes for ceremony entropy (Date.now is
predictable), add mkdirSync for build/, fix EJS template loading to
use readFileSync instead of dynamic import with assert (unsupported
in Node 20/22/24 ESM)
- circuits-ci.yml: pin circom to v2.2.3 instead of latest, add version
verification step
- KNOWN_ISSUES.md: fix misleading 'settlement-specific verifier' wording
— MARKPoolVerifier is a shared pool verifier, not settlement-specific
- MARKPool.sol: fix NatSpec EIP-712 reference to EIP-191 (personal_sign)
* feat(pool): add pool E2E test, fix RYLACreditLedger caller model
RYLACreditLedger:
- Separate credit (pool-only) and debit (adapter-only) callers
- Add setAdapter() one-time setter to break circular deploy dependency
(adapter constructor needs ledger, ledger needs adapter address)
- Add AdapterAlreadySet error
DeployMARKPool.s.sol:
- Call ledger.setAdapter(adapter) after adapter deployment
Tests:
- RYLACreditLedger.t.sol: updated for new caller model, 11 tests
- MARKWithdrawAdapter.t.sol: add setAdapter call in setUp
- MARKPoolE2E.t.sol: full withdrawal flow E2E test (3 tests)
- testFullWithdrawalFlow: mint RYLA -> transactWithWithdrawBinding
-> withdrawWithSig -> verify RYLA burned, ETH received
- testNullifierReplayRejected
- testBindingMismatchRejected
134/134 tests pass
* feat(pool): add ReleasePool.s.sol orchestrator and pool env vars
- ReleasePool.s.sol: release orchestrator for pool stack following the
same pattern as ReleaseMARK.s.sol — preflight checks, deploy via
DeployMARKPool, post-deploy verification (wiring checks + RYLA roles),
JSON artifact write
- .env.example: add pool stack env vars (MARK_POOL_VERIFIER,
MARK_POOL_OWNER, MARK_POOL_INTENT_SIGNER, release flags, artifact
path, post-deploy verify addresses)
* fix(pool): security fixes and dead code removal
RYLACreditLedger:
- Add OWNER immutable (set to msg.sender in constructor)
- Restrict setAdapter to OWNER to prevent front-running between
deployment and the setAdapter call in the release script
- Add testSetAdapterRevertsForNonOwner test
- Add clarifying NatSpec to totalCreditsOutstanding explaining it
tracks only flows through this ledger, not total RYLA supply
MARKWithdrawAdapter:
- Move ETH transfer before ASSET_LEDGER.debit — if ETH transfer
fails, RYLA is no longer burned (was a loss-of-funds bug)
MARKPool:
- Remove dead _seedRoot function (defined but never called)
- Add NatSpec to computePublicInputsWithWithdraw clarifying
chainId vs dstChainId semantics
* fix(test): fix nullifier replay test to use fresh signatures
testNullifierReplayRejected was reusing signatures computed for nonce N
in the second withdrawWithSig call with nonce N+1, causing a NonceMismatch
revert instead of exercising nullifier replay protection. Now recomputes
the intent hash and signs with the updated nonce so the revert is caused
by NullifierAlreadyClaimed as intended.
* fix(pool): guard totalCreditsOutstanding against underflow
* feat(pool): add pool release CI check and deploy script tests
contracts-ci.yml:
- Add pool release dry-run and execute smoke steps to the
contracts-release-check job, reusing the Anvil instance and
RYLA token deployed by the settlement release step
- Assert pool release artifact schema (pool, ledger, adapter addresses)
MARKPoolDeployScripts.t.sol:
- testDeployMARKPoolWiresAllContracts: verifies all contract wiring
(pool<->ledger, ledger<->adapter, RYLA roles)
- testDeployMARKPoolSetsIntentSignerWhenProvided: verifies intent signer
is configured when MARK_POOL_INTENT_SIGNER is set
- testDeployMARKPoolRevertsWhenMissingTokenAdmin: verifies preflight
check rejects deployer without RYLA admin role
138/138 tests pass
* fix(pool): address final CodeRabbit findings
- contracts-ci.yml: remove '|| true' from pool release dry-run step;
use the deployed settlement module address as verifier (a real contract)
so the preflight code.length check passes without masking failures
- RYLACreditLedger.sol: fix NatSpec on totalCreditsOutstanding to
accurately describe accounting scope — _totalBurned can exceed
_totalMinted if RYLA is burned via other paths (e.g. settlement module)
* fix(ci): fix pool release CI failure and address CodeRabbit finding
contracts-ci.yml:
- Add --skip-simulation to pool release broadcast — PoseidonT3 (55,856
bytes) exceeds EIP-170 limit and cannot be deployed without refactoring
to a linked library; --skip-simulation tests script orchestration only
- Fix jq assertion to use regex validation instead of zero-address check,
rejecting null values and validating hex address format
KNOWN_ISSUES.md:
- Add KI-8 documenting PoseidonT3 contract size issue and required fix
before mainnet (deploy as standalone contract, call via interface)
* fix(ci): remove pool execute smoke, fix jq assertion, fix KI-7 wording
contracts-ci.yml:
- Remove pool release execute smoke step — MARKPool (24,841 bytes) and
PoseidonT3 (55,856 bytes) exceed EIP-170 limit and cannot be broadcast
to Anvil; pool deploy requires PoseidonT3 refactor (KI-8) first
- Keep pool release dry-run only (validates script logic and preflight)
- Remove the now-unused artifact assertion step
KNOWN_ISSUES.md:
- Fix KI-7: both pool and settlement systems use the same MARKPool
circuit — remove implication of distinct circuit designs
* fix(pool): add code.length checks to RYLACreditLedger constructor and setAdapter
Prevents EOAs from being set as TOKEN, POOL, or ADAPTER.
Adds InvalidContract error. 3 new tests cover the EOA rejection cases.
setUp uses vm.etch to give mock addresses contract bytecode.
* fix(contracts): harden settlement verifier flow and CI reliability
* fix(review): address open CI and pool verifier feedback
* refactor(pool): rename min fee guard error for clarity
* fix(pool,settlement): replace require strings and wrong errors with custom errors
PoolFeePolicy:
- Replace require(maxFeeBurnBps != 0, string) and require(feeBurnBps <= maxFeeBurnBps, string)
with custom error FeePolicyInvalidBps() — consistent with codebase style, lower gas
Groth16SettlementVerifier:
- Replace ZeroAddress() with VerifierNotAContract() for verifierContract code.length check
- Replace ZeroAddress() with SettlementModuleNotAContract() for settlementModule code.length check
- ZeroAddress was semantically wrong for non-zero addresses that have no code
* ci: trigger fresh CI run
* docs(pool): correct KI-8 — MARKPool itself is over EIP-170 size limit
Investigation: MARKPool is 24,960 bytes (over 24,576 limit) even without
PoseidonT3 inlining. via_ir=true already prevents PoseidonT3 from being
inlined. The fix requires splitting MARKPool into smaller contracts, not
just extracting PoseidonT3 as a standalone contract. Both are required.
* fix(pool): reduce MARKPool below EIP-170 size limit (24200 < 24576 bytes)
Size reductions (24961 -> 24200 bytes, -761 bytes):
- Remove redundant verifierAddr.code.length check in _verifyAndConsume
(already validated in setVerifier, cannot change after deployment)
- Remove redundant tail != rootQueueTail guard in _insertCommitmentsValidated
(always true after inserting 2 commitments)
- Inline _requireCommitmentsValid wrapper (single-line delegation)
- Inline _insertCommitments wrapper (only called from bridgeIn)
- Remove computePublicInputs and computePublicInputsWithWithdraw public
view functions from MARKPool — _buildPublicInputs now calls
PoolPublicInputs.build directly; off-chain callers use PoolPublicInputs
Bug fixes:
- PoolValidation: move NullifierDuplicate check before the loop so
duplicate nullifiers get the precise error, not NullifierUsed
- MARKPool.pause(): document that unpause() does NOT auto-restore
withdrawals (intentional asymmetry, requires explicit unpauseWithdrawals)
* fix: address CodeRabbit findings (circuits, Makefile, architecture-guard)
circuits/test/MARKPool.test.mjs:
- Remove unused buildMerklePath helper (tests use buildTwoLeafRoot)
circuits/setup.mjs:
- Add r1cs existence check before trusted setup with clear error message
contracts/Makefile:
- Restore test-core to exclude invariant tests (--no-match-path)
so ci-fast remains fast as documented
contracts/script/ci/architecture-guard.sh:
- Tighten all four import regexes to handle optional leading whitespace
and any number of ../ segments (prevents bypass via indented imports
or deeper relative paths)
* fix: address remaining CodeRabbit findings
contracts/src/pool/MARKPool.sol:
- setVerifier: add code.length check (consistent with constructor)
circuits/test/MARKPool.test.mjs:
- expectFail: only treat constraint/assertion failures as PASS;
rethrow other errors so regressions surface
contracts/KNOWN_ISSUES.md:
- KI-7: separate design capability from configuration state for
settlement system wording
* fix(circuits): lowercase error message comparison in expectFail
* docs(deployment): add Groth16SettlementVerifier wiring step (Step 18)
Documents the two post-deploy calls required to activate ZK-based
settlement: setSettlementModule and setVerifierContract on
Groth16SettlementVerifier, then setVerifier on MARKSettlementModule.
AttestedSettlementVerifier remains the fallback until wiring is complete.
* fix(settlement): return false on malformed proof in Groth16SettlementVerifier (#101)
abi.decode reverts on malformed/short proof bytes, which propagated
through MARKSettlementModule as a raw error instead of VerificationFailed.
Fix: check proof.length == 672 before decoding (fixed ABI encoding size:
uint256[2]+uint256[2][2]+uint256[2]+uint256[13] = 64+128+64+416 = 672).
Malformed proofs now return false cleanly.
Tests: testVerifySettlementReturnsFalseForMalformedProof,
testVerifySettlementReturnsFalseForEmptyProof
* fix(ci): exclude integration tests from test-core target (#102)
test-core was running integration tests (which require supersim on port 9545)
because --no-match-path on the command line overrides foundry.toml's
no_match_path setting rather than adding to it.
Use brace glob to exclude both invariant and integration tests.
* fix(test): remove unverifiable cross-chain assertion from integration test (#103)
testBridgeToTransfersTokensCrossChain switched to fork B and checked the
recipient balance, but Foundry fork tests cannot simulate supersim's async
message relay — the contract simply doesn't exist on the other fork.
Fix: assert only the source-chain burn (which is fully verifiable in a fork
test). Add a NatSpec note explaining the relay limitation.
* docs(pool): correct KI-8 — PoseidonT3 inlined via via_ir, MARKPool deployable (#104)
* docs(pool): correct KI-8 — PoseidonT3 is inlined via via_ir, MARKPool is deployable
via_ir=true causes the compiler to inline PoseidonT3 into MARKPool rather
than deploying it as a linked library. MARKPool has no link references and
is 24,298 bytes (278 bytes under EIP-170). KI-8 was based on an earlier
state where MARKPool exceeded the limit.
Updated KI-8 to reflect accurate current state and note the tight margin.
* refactor(crypto): use >>= 1 instead of /= 2 in MerkleTree insert
* security: harden pool domain before testnet (#105)
* security: harden pool domain before testnet
- Add pool/withdraw/Groth16 contracts to slither-core scope
- Document all slither exclusion rationale in Makefile
- RYLACreditLedger: add Credit/Debit events, move before external calls (CEI)
- MARKWithdrawAdapter: add test for recipient zero-check (existing check, missing test)
- THREAT_MODEL.md: add pool stack overview, trust boundaries, role compromise
impact, and 3 new invariants (nullifier replay, withdraw binding, debit approval)
* fix(ci): use per-contract slither exclusions instead of global
CodeRabbit correctly noted that global exclusions could suppress actionable
findings in newly added contracts. Refactored slither-core to apply only
the relevant exclusions per contract. Also added arbitrary-send-erc20 to
MARKSettlementModule and RYLACreditLedger (both use safeTransferFrom with
prior approval — not arbitrary).
* fix(ci): add set -e to slither-core, fix preflight to use python3 -m slither
Without set -e, a failing early slither invocation would be masked if the
final command succeeds. Also align the preflight check with the actual
invocation (python3 -m slither, not command -v slither).
* ci: fix 4 workflow issues pre-testnet (#106)
* ci: fix 4 workflow issues pre-testnet
1. Sync _reusable-contracts-slither.yml with Makefile
- Delegate to 'make slither-core' (single source of truth)
- Now covers all 8 contracts with per-contract exclusions
- Previously only scanned 4 settlement contracts with global exclusions
2. Enable pool execute smoke in contracts-ci.yml
- KI-8 resolved: via_ir inlines PoseidonT3, MARKPool is 24,298 bytes
- Pool broadcast to Anvil now works; remove stale blocker comment
3. Fix integration test readiness check
- Wait on ports 9545/9546 (actual RPC ports) not 8420 (admin port)
- Use nc loop consistent with anvil readiness pattern
4. Pin foundry-rs/foundry-toolchain to v1.8.0 commit SHA
- Floating @v1 could silently break on Foundry breaking changes
- Pinned: c7450ba673e133f5ee30098b3b54f444d3a2ca2d (v1.8.0)
* fix(ci): remove foundry version input from reusable slither workflow
The version input was passed as 'v1.8.0' to the action's 'version' input
which expects a Foundry binary tag (e.g. 'stable', 'nightly'), not the
action version. This caused foundryup to fail extracting the tar archive.
Use the action's default Foundry version instead.
* fix(ci): revert pool execute smoke — Foundry rejects PoseidonT3 artifact size
forge create/broadcast checks all library artifacts for EIP-170 compliance.
PoseidonT3 is 55,856 bytes as a standalone artifact even though via_ir inlines
it into MARKPool at compile time. The broadcast is blocked before deployment.
Keep dry-run only. Update KI-8 with the precise diagnosis.
* fix(pool): resolve PoseidonT3 deployment blocker via external interface (#107)
PoseidonT3 is a Solidity library with a public function — it gets deployed
as a separate linked contract (55,856 bytes) which exceeds EIP-170 (24,576).
This blocked all pool deployments.
Fix: replace the library call with an external interface (IPoseidonT3).
MerkleTree now stores the Poseidon contract address in the Tree struct and
calls it via DELEGATECALL-free external call. MARKPool constructor accepts
a _poseidon address parameter.
Default deployment address: 0xB43122Ecb241DD50062641f089876679fd06599a
This is Semaphore's PoseidonT3 (PSE/Ethereum Foundation), deployed at the
same address on all EVM networks via CREATE2. Verified compatible with our
implementation: hash([0,0]) and hash([1,2]) produce identical outputs.
MARKPool now has zero link references and is fully self-contained.
MARKPool size: 24,231 bytes (345 bytes margin under EIP-170).
Tests: deployCode('PoseidonT3.sol:PoseidonT3') in test setUp bypasses
EIP-170 (Foundry test runner does not enforce the limit).
* chore(circuits): remove stale UTXOSettlement artifacts (#108)
* chore(circuits): remove stale UTXOSettlement artifacts
UTXOSettlement circuit is superseded by MARKPool.circom.
Remove the stale test file and old verification key artifact.
The utxo/ source and build/ artifacts are already gitignored.
* ci: trigger Release Gate Container for circuits-only PRs
Add circuits/** to path filter so the required check runs and passes
when only circuit files change (no contracts affected).
* ci: add circuits/** to push paths for consistency
* ci: remove path filter from release gate pull_request trigger
* ci: add circuits/** to CodeQL path filter to unblock circuits-only PRs
* fix: address codebase review findings (#109)
Bug: RYLACreditLedger.debit() — move _totalBurned update before
safeTransferFrom to follow CEI pattern. Previously the state update
happened after the external call, creating a reentrancy window where
_totalBurned was not yet incremented during the transfer callback.
Docs: KNOWN_ISSUES.md KI-8 — update stale size figures and description.
MARKPool is now 24,231 bytes (345 bytes margin). PoseidonT3 is no longer
inlined via via_ir; MerkleTree calls it via IPoseidonT3 interface at
0xB43122... (Semaphore, same address on all EVM networks).
Tests: add testConstructorRevertsOnZeroPoseidon and
testConstructorRevertsOnEOAPoseidon to MARKPool.t.sol — the _poseidon
constructor parameter added in PR #107 had no test coverage.
* ci: pin action-shellcheck to commit SHA (#110)
* ci: pin action-shellcheck to commit SHA
ludeeus/action-shellcheck@2.0.0 was pinned by version tag only.
Tags are mutable — a compromised tag could point to malicious code.
Pin to the immutable commit SHA (00cae50) for supply chain safety.
* ci: trigger CodeQL for all .github/workflows/** changes
* chore(deps): bump actions/dependency-review-action from 4 to 5 (#90)
Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 4 to 5.
- [Release notes](https://github.com/actions/dependency-review-action/releases)
- [Commits](https://github.com/actions/dependency-review-action/compare/v4...v5)
---
updated-dependencies:
- dependency-name: actions/dependency-review-action
dependency-version: '5'
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore(deps): bump the frontend-minor-patch group across 1 directory with 21 updates (#91)
Bumps the frontend-minor-patch group with 6 updates in the / directory:
| Package | From | To |
| --- | --- | --- |
| [@tailwindcss/vite](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/@tailwindcss-vite) | `4.2.4` | `4.3.0` |
| [tailwind-merge](https://github.com/dcastil/tailwind-merge) | `3.5.0` | `3.6.0` |
| [tailwindcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/tailwindcss) | `4.2.4` | `4.3.0` |
| [baseline-browser-mapping](https://github.com/web-platform-dx/baseline-browser-mapping) | `2.10.27` | `2.10.29` |
| [electron-to-chromium](https://github.com/Kilian/electron-to-chromium) | `1.5.352` | `1.5.353` |
| [get-east-asian-width](https://github.com/sindresorhus/get-east-asian-width) | `1.5.0` | `1.6.0` |
Updates `@tailwindcss/vite` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/packages/@tailwindcss-vite)
Updates `tailwind-merge` from 3.5.0 to 3.6.0
- [Release notes](https://github.com/dcastil/tailwind-merge/releases)
- [Commits](https://github.com/dcastil/tailwind-merge/compare/v3.5.0...v3.6.0)
Updates `tailwindcss` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/packages/tailwindcss)
Updates `@tailwindcss/node` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/packages/@tailwindcss-node)
Updates `@tailwindcss/oxide-android-arm64` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/android-arm64)
Updates `@tailwindcss/oxide-darwin-arm64` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/darwin-arm64)
Updates `@tailwindcss/oxide-darwin-x64` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/darwin-x64)
Updates `@tailwindcss/oxide-freebsd-x64` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/freebsd-x64)
Updates `@tailwindcss/oxide-linux-arm-gnueabihf` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-arm-gnueabihf)
Updates `@tailwindcss/oxide-linux-arm64-gnu` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-arm64-gnu)
Updates `@tailwindcss/oxide-linux-arm64-musl` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-arm64-musl)
Updates `@tailwindcss/oxide-linux-x64-gnu` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-x64-gnu)
Updates `@tailwindcss/oxide-linux-x64-musl` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-x64-musl)
Updates `@tailwindcss/oxide-wasm32-wasi` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node)
Updates `@tailwindcss/oxide-win32-arm64-msvc` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/win32-arm64-msvc)
Updates `@tailwindcss/oxide-win32-x64-msvc` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/win32-x64-msvc)
Updates `@tailwindcss/oxide` from 4.2.4 to 4.3.0
- [Release notes](https://github.com/tailwindlabs/tailwindcss/releases)
- [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md)
- [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node)
Updates `baseline-browser-mapping` from 2.10.27 to 2.10.29
- [Release notes](https://github.com/web-platform-dx/baseline-browser-mapping/releases)
- [Commits](https://github.com/web-platform-dx/baseline-browser-mapping/compare/v2.10.27...v2.10.29)
Updates `electron-to-chromium` from 1.5.352 to 1.5.353
- [Changelog](https://github.com/Kilian/electron-to-chromium/blob/main/CHANGELOG.md)
- [Commits](https://github.com/Kilian/electron-to-chromium/compare/v1.5.352...v1.5.353)
Updates `enhanced-resolve` from 5.21.1 to 5.21.2
- [Release notes](https://github.com/webpack/enhanced-resolve/releases)
- [Changelog](https://github.com/webpack/enhanced-resolve/blob/main/CHANGELOG.md)
- [Commits](https://github.com/webpack/enhanced-resolve/compare/v5.21.1...v5.21.2)
Updates `get-east-asian-width` from 1.5.0 to 1.6.0
- [Release notes](https://github.com/sindresorhus/get-east-asian-width/releases)
- [Commits](https://github.com/sindresorhus/get-east-asian-width/compare/v1.5.0...v1.6.0)
---
updated-dependencies:
- dependency-name: "@tailwindcss/node"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-android-arm64"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-darwin-arm64"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-darwin-x64"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-freebsd-x64"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-linux-arm-gnueabihf"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-linux-arm64-gnu"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-linux-arm64-musl"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-linux-x64-gnu"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-linux-x64-musl"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-wasm32-wasi"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-win32-arm64-msvc"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/oxide-win32-x64-msvc"
dependency-version: 4.3.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: "@tailwindcss/vite"
dependency-version: 4.3.0
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: baseline-browser-mapping
dependency-version: 2.10.29
dependency-type: indirect
update-type: version-update:semver-patch
dependency-group: frontend-minor-patch
- dependency-name: electron-to-chromium
dependency-version: 1.5.353
dependency-type: indirect
update-type: version-update:semver-patch
dependency-group: frontend-minor-patch
- dependency-name: enhanced-resolve
dependency-version: 5.21.2
dependency-type: indirect
update-type: version-update:semver-patch
dependency-group: frontend-minor-patch
- dependency-name: get-east-asian-width
dependency-version: 1.6.0
dependency-type: indirect
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: tailwind-merge
dependency-version: 3.6.0
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
- dependency-name: tailwindcss
dependency-version: 4.3.0
dependency-type: direct:production
update-type: version-update:semver-minor
dependency-group: frontend-minor-patch
...
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Iko <6572003+iap@users.noreply.github.com>
* chore: update LICENSE copyright to Trade 2026 (#111)
* chore: update LICENSE copyright to Trade 2026
The project was scaffolded from an Optimism template but is original work.
Update copyright holder from Optimism to Trade and year to 2026.
* ci: remove path filter from CodeQL pull_request trigger
CodeQL is a required check for all PRs. With a path filter, PRs that
only touch files outside the filter (e.g. LICENSE, README) are blocked
indefinitely waiting for CodeQL results that never come.
Remove the pull_request path filter so CodeQL always runs on PRs.
Keep the push path filter to avoid unnecessary runs on branch pushes.
* chore: remove stale deploy-contracts step from mprocs.yaml (#112)
deploy:supersim and deploy:counter-incrementer:supersim are template
artifacts from the original Optimism scaffold. They no longer exist.
Remove the stale deploy-contracts proc.
---------
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced May 17, 2026
iap
added a commit
that referenced
this pull request
May 17, 2026
…116) * ops: enforce promotion freshness and commit lineage checks * ops: add full release evidence dispatch sequence * ops: add release secret bootstrap helper * ops: harden release dispatch run correlation and strict env checks * refactor: centralize core contract custom errors * refactor(contracts): separate bridge/settlement domains and harden settlement flows * ci(contracts): enforce architecture/layering guards and fix refactor paths * chore(contracts): remove legacy files replaced by domain refactor * feat(ops): add canonical release-gate workflow with evidence artifact * chore(governance): align release flow and policy guard with canary promotion 1. Update release PR template to canary -> main 2. Extend governance-policy-guard push triggers to include canary 3. Document explicit release promotion path dev -> canary -> main in root README 4. Clarify retirement of legacy CrossChainCounter examples/tests in contracts README 5. Keep governance consistency validator passing after updates * feat(release): harden CI gates and retire cross-chain demo artifacts 1. Enable canary across contracts CI, env guard, slither, and secrets drift workflows 2. Add canary push-driven staging rehearsal defaults and stricter required input checks 3. Strengthen release gate with signed evidence-manifest verification and artifact-anchored deployment verification 4. Add evidence tooling scripts (generate/sign/verify manifest and signature, verify-from-artifact) 5. Retire legacy CrossChainCounter example contracts, ABIs, deploy script, and associated tests 6. Update app shell and package scripts toward MARK protocol operations workflow * chore(ci): stabilize local test and lint signal 1. Exclude vendored/generated contract directories from root ESLint scope 2. Split fast core tests and invariant tests in contracts Makefile 3. Bound local invariant runs for predictable ci-full runtime * fix(ci): repair contracts workflow execution on GitHub 1. Fix contracts integration job condition to use github.event.inputs 2. Run slither per target contract instead of invalid multi-target invocation * fix(ci): quote static private key in contracts-ci workflow env * fix(slither): codify accepted detector exclusions for MARK contracts Exclude known/accepted findings (naming convention, timestamp epoching, operator-gated transferFrom pattern, and benign reentrancy patterns) while keeping fail-medium enforcement for remaining detectors. * chore(ci): harden workflow runtime compatibility and add frontend node matrix 1. Upgrade actions/checkout from v4 to v5 across workflows 2. Upgrade actions/setup-python from v5 to v6 in python-based workflows 3. Add frontend CI workflow with Node 20/22 matrix for typecheck, lint, and build validation * fix(frontend-ci): ensure pnpm setup works with node matrix Use actions/setup-node@v5 and remove premature pnpm cache wiring so pnpm/action-setup can install pnpm before dependency install. * fix(frontend-ci): install pnpm before setup-node auto-cache check * fix(frontend-ci): rely on packageManager-pinned pnpm version * chore(ci): replace pnpm action with corepack-pinned bootstrap 1. Remove pnpm/action-setup usage from frontend and contracts integration workflows 2. Use corepack with pinned pnpm@9.0.2 from project policy 3. Disable setup-node package-manager auto-cache probing to avoid pnpm bootstrap race * fix(contracts-ci): wait for anvil before release dry-run * chore(deps): add dependabot config for actions and npm * chore(deps): add dependabot config for actions and npm * chore(coderabbit): add repository-level review configuration * fix(readiness): run pre-checks before contracts working directory exists * chore: promote dev to canary (ci and quality sync) (#15) * chore(deps): bump actions/setup-node from 5 to 6 Bumps [actions/setup-node](https://github.com/actions/setup-node) from 5 to 6. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](https://github.com/actions/setup-node/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump actions/upload-artifact from 4 to 7 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 7. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v4...v7) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump actions/checkout from 5 to 6 Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump actions/github-script from 7 to 9 Bumps [actions/github-script](https://github.com/actions/github-script) from 7 to 9. - [Release notes](https://github.com/actions/github-script/releases) - [Commits](https://github.com/actions/github-script/compare/v7...v9) --- updated-dependencies: - dependency-name: actions/github-script dependency-version: '9' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump the frontend-minor-patch group with 13 updates Bumps the frontend-minor-patch group with 13 updates: | Package | From | To | | --- | --- | --- | | [@eth-optimism/viem](https://github.com/ethereum-optimism/ecosystem/tree/HEAD/packages/viem) | `0.3.2` | `0.4.15` | | [@radix-ui/react-separator](https://github.com/radix-ui/primitives) | `1.1.2` | `1.1.8` | | [@radix-ui/react-slot](https://github.com/radix-ui/primitives) | `1.1.2` | `1.2.4` | | [@tailwindcss/vite](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/@tailwindcss-vite) | `4.0.6` | `4.2.4` | | [@tanstack/react-query](https://github.com/TanStack/query/tree/HEAD/packages/react-query) | `5.66.0` | `5.100.8` | | [abitype](https://github.com/wevm/abitype) | `1.0.8` | `1.2.4` | | [tailwind-merge](https://github.com/dcastil/tailwind-merge) | `3.0.1` | `3.5.0` | | [tailwindcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/tailwindcss) | `4.0.6` | `4.2.4` | | [viem](https://github.com/wevm/viem) | `2.23.1` | `2.48.8` | | [eslint-plugin-react-refresh](https://github.com/ArnaudBarre/eslint-plugin-react-refresh) | `0.4.19` | `0.5.2` | | [mprocs](https://github.com/pvolok/mprocs) | `0.7.2` | `0.9.2` | | [prettier](https://github.com/prettier/prettier) | `3.5.0` | `3.8.3` | | [typescript-eslint](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint) | `8.24.0` | `8.59.1` | Updates `@eth-optimism/viem` from 0.3.2 to 0.4.15 - [Changelog](https://github.com/ethereum-optimism/ecosystem/blob/main/packages/viem/CHANGELOG.md) - [Commits](https://github.com/ethereum-optimism/ecosystem/commits/HEAD/packages/viem) Updates `@radix-ui/react-separator` from 1.1.2 to 1.1.8 - [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md) - [Commits](https://github.com/radix-ui/primitives/commits) Updates `@radix-ui/react-slot` from 1.1.2 to 1.2.4 - [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md) - [Commits](https://github.com/radix-ui/primitives/commits) Updates `@tailwindcss/vite` from 4.0.6 to 4.2.4 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.2.4/packages/@tailwindcss-vite) Updates `@tanstack/react-query` from 5.66.0 to 5.100.8 - [Release notes](https://github.com/TanStack/query/releases) - [Changelog](https://github.com/TanStack/query/blob/main/packages/react-query/CHANGELOG.md) - [Commits](https://github.com/TanStack/query/commits/@tanstack/react-query@5.100.8/packages/react-query) Updates `abitype` from 1.0.8 to 1.2.4 - [Release notes](https://github.com/wevm/abitype/releases) - [Commits](https://github.com/wevm/abitype/compare/abitype@1.0.8...abitype@1.2.4) Updates `tailwind-merge` from 3.0.1 to 3.5.0 - [Release notes](https://github.com/dcastil/tailwind-merge/releases) - [Commits](https://github.com/dcastil/tailwind-merge/compare/v3.0.1...v3.5.0) Updates `tailwindcss` from 4.0.6 to 4.2.4 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.2.4/packages/tailwindcss) Updates `viem` from 2.23.1 to 2.48.8 - [Release notes](https://github.com/wevm/viem/releases) - [Commits](https://github.com/wevm/viem/compare/viem@2.23.1...viem@2.48.8) Updates `eslint-plugin-react-refresh` from 0.4.19 to 0.5.2 - [Release notes](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/releases) - [Changelog](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/blob/main/CHANGELOG.md) - [Commits](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/compare/v0.4.19...v0.5.2) Updates `mprocs` from 0.7.2 to 0.9.2 - [Release notes](https://github.com/pvolok/mprocs/releases) - [Changelog](https://github.com/pvolok/mprocs/blob/master/CHANGELOG.md) - [Commits](https://github.com/pvolok/mprocs/compare/v0.7.2...v0.9.2) Updates `prettier` from 3.5.0 to 3.8.3 - [Release notes](https://github.com/prettier/prettier/releases) - [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md) - [Commits](https://github.com/prettier/prettier/compare/3.5.0...3.8.3) Updates `typescript-eslint` from 8.24.0 to 8.59.1 - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.59.1/packages/typescript-eslint) --- updated-dependencies: - dependency-name: "@eth-optimism/viem" dependency-version: 0.4.15 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: "@radix-ui/react-separator" dependency-version: 1.1.8 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: frontend-minor-patch - dependency-name: "@radix-ui/react-slot" dependency-version: 1.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: "@tailwindcss/vite" dependency-version: 4.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: "@tanstack/react-query" dependency-version: 5.100.8 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: abitype dependency-version: 1.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: tailwind-merge dependency-version: 3.5.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: tailwindcss dependency-version: 4.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: viem dependency-version: 2.48.8 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: eslint-plugin-react-refresh dependency-version: 0.5.2 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: mprocs dependency-version: 0.9.2 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: prettier dependency-version: 3.8.3 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: typescript-eslint dependency-version: 8.59.1 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com> * fix(readiness): run pre-checks before contracts working directory exists * fix(frontend): remove non-component export from button ui --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * ci(security): add codeql and dependency review gates * chore: promote dev to canary 56 commits: EIP-712 verifier, bridge tests, CI fixes, governance cleanup, trust model doc, RYLA Credits rename. * chore: promote dev to canary 65 commits: staging rehearsal fixes, frontend info page, NatSpec improvements, README philosophy. * chore: promote dev to canary (v0.1.1 prep) 69 commits: CEI fix, audit docs, staging pipeline fixes, frontend info page. * chore: promote dev to canary Promotes dev to canary. Staging rehearsal will trigger on merge. * chore: promote dev to canary Promotes dev to canary. Staging rehearsal will trigger on merge. * chore: promote dev to canary Promotes dev to canary. Includes --slow fix for staging rehearsal. * chore: promote dev to canary Promotes dev to canary. Governance fixes: 0 approvals, push restrictions to trade/maintainers, verify-governance.sh checks. * chore: promote dev to canary Promotes dev to canary. Includes Groth16SettlementVerifier, IGroth16Verifier, dependabot fix, governance push restrictions. * chore: promote dev to canary for OP Sepolia staging (#114) * chore(deps): bump actions/setup-node from 5 to 6 Bumps [actions/setup-node](https://github.com/actions/setup-node) from 5 to 6. - [Release notes](https://github.com/actions/setup-node/releases) - [Commits](https://github.com/actions/setup-node/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/setup-node dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump actions/upload-artifact from 4 to 7 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 7. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v4...v7) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump actions/checkout from 5 to 6 Bumps [actions/checkout](https://github.com/actions/checkout) from 5 to 6. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](https://github.com/actions/checkout/compare/v5...v6) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '6' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump actions/github-script from 7 to 9 Bumps [actions/github-script](https://github.com/actions/github-script) from 7 to 9. - [Release notes](https://github.com/actions/github-script/releases) - [Commits](https://github.com/actions/github-script/compare/v7...v9) --- updated-dependencies: - dependency-name: actions/github-script dependency-version: '9' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * chore(deps): bump the frontend-minor-patch group with 13 updates Bumps the frontend-minor-patch group with 13 updates: | Package | From | To | | --- | --- | --- | | [@eth-optimism/viem](https://github.com/ethereum-optimism/ecosystem/tree/HEAD/packages/viem) | `0.3.2` | `0.4.15` | | [@radix-ui/react-separator](https://github.com/radix-ui/primitives) | `1.1.2` | `1.1.8` | | [@radix-ui/react-slot](https://github.com/radix-ui/primitives) | `1.1.2` | `1.2.4` | | [@tailwindcss/vite](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/@tailwindcss-vite) | `4.0.6` | `4.2.4` | | [@tanstack/react-query](https://github.com/TanStack/query/tree/HEAD/packages/react-query) | `5.66.0` | `5.100.8` | | [abitype](https://github.com/wevm/abitype) | `1.0.8` | `1.2.4` | | [tailwind-merge](https://github.com/dcastil/tailwind-merge) | `3.0.1` | `3.5.0` | | [tailwindcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/tailwindcss) | `4.0.6` | `4.2.4` | | [viem](https://github.com/wevm/viem) | `2.23.1` | `2.48.8` | | [eslint-plugin-react-refresh](https://github.com/ArnaudBarre/eslint-plugin-react-refresh) | `0.4.19` | `0.5.2` | | [mprocs](https://github.com/pvolok/mprocs) | `0.7.2` | `0.9.2` | | [prettier](https://github.com/prettier/prettier) | `3.5.0` | `3.8.3` | | [typescript-eslint](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/typescript-eslint) | `8.24.0` | `8.59.1` | Updates `@eth-optimism/viem` from 0.3.2 to 0.4.15 - [Changelog](https://github.com/ethereum-optimism/ecosystem/blob/main/packages/viem/CHANGELOG.md) - [Commits](https://github.com/ethereum-optimism/ecosystem/commits/HEAD/packages/viem) Updates `@radix-ui/react-separator` from 1.1.2 to 1.1.8 - [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md) - [Commits](https://github.com/radix-ui/primitives/commits) Updates `@radix-ui/react-slot` from 1.1.2 to 1.2.4 - [Changelog](https://github.com/radix-ui/primitives/blob/main/release-process.md) - [Commits](https://github.com/radix-ui/primitives/commits) Updates `@tailwindcss/vite` from 4.0.6 to 4.2.4 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.2.4/packages/@tailwindcss-vite) Updates `@tanstack/react-query` from 5.66.0 to 5.100.8 - [Release notes](https://github.com/TanStack/query/releases) - [Changelog](https://github.com/TanStack/query/blob/main/packages/react-query/CHANGELOG.md) - [Commits](https://github.com/TanStack/query/commits/@tanstack/react-query@5.100.8/packages/react-query) Updates `abitype` from 1.0.8 to 1.2.4 - [Release notes](https://github.com/wevm/abitype/releases) - [Commits](https://github.com/wevm/abitype/compare/abitype@1.0.8...abitype@1.2.4) Updates `tailwind-merge` from 3.0.1 to 3.5.0 - [Release notes](https://github.com/dcastil/tailwind-merge/releases) - [Commits](https://github.com/dcastil/tailwind-merge/compare/v3.0.1...v3.5.0) Updates `tailwindcss` from 4.0.6 to 4.2.4 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.2.4/packages/tailwindcss) Updates `viem` from 2.23.1 to 2.48.8 - [Release notes](https://github.com/wevm/viem/releases) - [Commits](https://github.com/wevm/viem/compare/viem@2.23.1...viem@2.48.8) Updates `eslint-plugin-react-refresh` from 0.4.19 to 0.5.2 - [Release notes](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/releases) - [Changelog](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/blob/main/CHANGELOG.md) - [Commits](https://github.com/ArnaudBarre/eslint-plugin-react-refresh/compare/v0.4.19...v0.5.2) Updates `mprocs` from 0.7.2 to 0.9.2 - [Release notes](https://github.com/pvolok/mprocs/releases) - [Changelog](https://github.com/pvolok/mprocs/blob/master/CHANGELOG.md) - [Commits](https://github.com/pvolok/mprocs/compare/v0.7.2...v0.9.2) Updates `prettier` from 3.5.0 to 3.8.3 - [Release notes](https://github.com/prettier/prettier/releases) - [Changelog](https://github.com/prettier/prettier/blob/main/CHANGELOG.md) - [Commits](https://github.com/prettier/prettier/compare/3.5.0...3.8.3) Updates `typescript-eslint` from 8.24.0 to 8.59.1 - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/typescript-eslint/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v8.59.1/packages/typescript-eslint) --- updated-dependencies: - dependency-name: "@eth-optimism/viem" dependency-version: 0.4.15 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: "@radix-ui/react-separator" dependency-version: 1.1.8 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: frontend-minor-patch - dependency-name: "@radix-ui/react-slot" dependency-version: 1.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: "@tailwindcss/vite" dependency-version: 4.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: "@tanstack/react-query" dependency-version: 5.100.8 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: abitype dependency-version: 1.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: tailwind-merge dependency-version: 3.5.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: tailwindcss dependency-version: 4.2.4 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: viem dependency-version: 2.48.8 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: eslint-plugin-react-refresh dependency-version: 0.5.2 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: mprocs dependency-version: 0.9.2 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: prettier dependency-version: 3.8.3 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch - dependency-name: typescript-eslint dependency-version: 8.59.1 dependency-type: direct:development update-type: version-update:semver-minor dependency-group: frontend-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com> * fix(readiness): run pre-checks before contracts working directory exists * fix(frontend): remove non-component export from button ui * ci(security): add codeql and dependency review gates * chore(security): add local slither install and core scan targets * docs(phase1): add comprehensive contributor & deployment runbooks Add Phase 1 foundation documentation for team scaling and professional maintenance: CONTRIBUTING.md: - Local development setup instructions (Node, Foundry, super-cli) - Feature branch workflow with conventional commits - Code standards (TypeScript, Solidity, Testing) - PR submission checklist and review process - Testing guidelines and test structure - Troubleshooting for common dev issues DEPLOYMENT.md: - Step-by-step staging deployment runbook (OP Sepolia) - Mainnet deployment procedures with gates - Pre/post-deployment checklists - Evidence generation and verification - Monitoring and health checks - Rollback procedures for emergency scenarios - Comprehensive troubleshooting guide - Command cheat sheet and timeline estimates TROUBLESHOOTING.md: - Development setup issues (pnpm, Node, Foundry, super-cli, git hooks) - Smart contract issues (architecture guard, layering guard, Slither findings) - Frontend development issues (port conflicts, TypeScript errors, module resolution) - Testing issues (hanging tests, gas, balance) - Deployment issues (insufficient funds, timeouts, RPC problems) - CI/CD workflow issues (stuck workflows, secrets, version mismatches) - Network & RPC issues (timeouts, contract not found, chain ID) .github/CODEOWNERS: - Enhanced documentation with clear sections - Added review requirements annotations - Better organization for team scaling - Maintains strict single-owner model (ready for multi-owner when scaling) Impact: - Enables solo maintainer to self-document workflows - Provides clear onboarding path for new contributors - Establishes professional deployment procedures - Reduces support burden with comprehensive troubleshooting - Foundation for team collaboration (docs ready for team addition) - Production-ready documentation for auditors and stakeholders This commit fulfills Phase 1 foundation requirements: ✅ CONTRIBUTING.md created ✅ DEPLOYMENT.md runbook created ✅ TROUBLESHOOTING.md created ✅ CODEOWNERS enhanced and documented Ready for: Phase 2 (interactive UI) and Phase 3 (security audit planning) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * chore(deps): bump github/codeql-action from 3 to 4 (#16) Bump github/codeql-action from v3 to v4 to resolve Node.js 20 deprecation warnings on CI. * chore(ci): bump dependency-review-action from v4 to v5 * chore(ci): disable CodeQL triggers until repo transferred to org with GHAS * Enable org-transfer governance: CodeQL, Gitleaks, release-gate container, and verification scripts (#19) * docs: replace roadmap with lean security next-steps guide * fix(docs): remove duplicate required-check entries in BRANCHING.md * fix(ci): add USER root in release-gate Dockerfile for apt-get permissions * ci(security): fix dependency review tag and use OSS gitleaks CLI * ci(security): fix gitleaks PATH on github runner * ci(security): run gitleaks scan via docker image * ci(security): remove hardcoded key and scope gitleaks to workspace * ci(contracts): fix anvil key extraction for release check * ci(contracts): require 64-byte anvil private key extraction * ci: always run contracts/frontend checks on protected branches (#21) * ci: phase-1 reusable workflows for frontend, slither, and secrets scan (#23) * ci: extract reusable frontend/slither/secrets workflows * ci(security): apply codereview pinning and permissions fixes * fix(contracts): bridge approval safety + IRYLA interface decoupling - Wrap sendERC20 in try/catch; clear approval and revert with BridgeFailed() on failure - Extract IRYLA interface (inherits IERC20); MARKSettlementModule decoupled from concrete RYLA type - Add unit test for BridgeFailed catch branch * docs: sync governance and CI docs with current protections - Add missing required checks (Secrets Drift Guard, Release Gate Container) to all branch matrices - Fix Analyze (JavaScript/TypeScript) casing to match canonical check names - Fixes Validate Governance Policy Consistency CI check * chore(deps): bump frontend minor/patch dependencies 105 minor and patch updates including: - @tanstack/react-query 5.100.8 → 5.100.9 - typescript-eslint 8.59.1 → 8.59.2 - bufferutil 4.0.9 → 4.1.0 - jiti 2.6.1 → 2.7.0 - lockfile resolutions updated accordingly All CI checks pass on Node 20 and 22. * fix(deps): bump vite 6.1.0 → 6.4.2 (security) Fixes high-severity arbitrary file read CVE and medium-severity path traversal in vite dev server. * test(contracts): add missing unit test coverage 71 tests (was 59). Covers zero-input guards, exact error selectors, accumulator resets, supportsInterface, and isMint flag binding. * chore(governance): migrate CODEOWNERS to @trade/maintainers team Replaces @iap with @trade/maintainers across all CODEOWNERS entries. Team created with maintain permission on repo. * chore(ci): switch CodeRabbit to assertive profile profile: chill → assertive, request_changes_workflow: false → true * fix(docs): add VALIDATE_MODE to staging checklist prerequisites Adds missing VALIDATE_MODE env var to staging checklist. Clarifies operator/attester rotation step with RUNBOOK.md reference. Removes trailing newline from package.json. * chore(docs): remove stale pre-transfer planning documents Removes TRANSFER_NOW_CHECKLIST.md, ORG_TRANSFER_SECURITY_CHECKLIST.md, SECURITY_NEXT_STEPS.md, PROJECT_REVIEW.md — all completed with the org transfer on May 6, 2026. * chore(governance): clean up CODEOWNERS Remove decorative section dividers, redundant comments, and duplicate entry. Consolidate contract path globs. * fix(ci): workflow correctness and consistency fixes Pin slither-analyzer==0.11.5, fix secrets-drift-guard false positives, fix verify-governance.sh dismiss_stale_reviews on dev, add canary to evidence-manifest trigger, fix inputs context, fix wait-port, add pull_request_target comments, add Docker layer caching. * feat(contracts): migrate AttestedSettlementVerifier to EIP-712 Replace hybrid EIP-191 pattern with standard EIP-712 typed data signing. Expose settlementDigest() for off-chain signers. Add NatSpec on proof encoding and contextHash. 71 tests pass. * chore: improve gitignore coverage Add .env/.env.*/*.env and supersim-logs/ to root gitignore. Add coverage/ to contracts gitignore. * fix(ci): reliability and correctness fixes Add timeout-minutes:15 to stuck jobs, replace rg with grep -Eo in smoke script, pin slither==0.11.5 in Makefile, add explicit invariant runs=256 to foundry.toml. * chore(deps): ignore transitive alerts from super-cli Ignore @hono/node-server, drizzle-orm, @stablelib/ed25519 scoped to vulnerable versions — all transitive from super-cli dev tool, no upstream fix available. * docs: add SECURITY.md Reporting channel, scope, response SLA, and supported versions. * chore(deps): bump @types/node from 22.13.1 to 25.6.1 Type definitions update. * chore(deps): bump typescript from 5.7.3 to 6.0.3 Add ignoreDeprecations:6.0 for baseUrl deprecation warning. * chore(deps): bump frontend-minor-patch group viem, debug, and other minor/patch updates. * chore(deps): bump docker/setup-buildx-action from 3 to 4 Node 24 runtime update. * chore(deps): bump frontend-minor-patch group Minor/patch frontend dependency updates. * fix: stale references and check name mismatches Remove chainId double-encoding from AttestedSettlementVerifier, fix stale iap/mark URLs, fix governance script check names to match actual CI output. * test(contracts): add bridge integration test against supersim Exercises MARKBridgeAdapter against live SuperchainTokenBridge on two supersim forks. Verifies cross-chain token transfer and rate limit enforcement. * test(contracts): add bridge adapter invariant fuzz tests Three invariants covering rate limiting: daily cap never exceeded, accumulator consistent with cap, zero address never holds operator role. 74 tests pass. * fix(governance): sync check lists and fix ruleset condition Fix ruleset condition bug (canary/main now covered), sync apply-governance.sh and verify-governance.sh with live branch protection, fix frontend check name prefix in docs. * chore(governance): document new ruleset structure Two focused rulesets: branch-protection (CodeQL alert gate) and tag-protection (v* tags). Replaces the broken develop ruleset. * feat(token): rename RYLA display name to 'RYLA Credits' name() returns 'RYLA Credits', symbol stays 'RYLA'. Test and verification script updated. * test Documents key roles and trust assumptions, attester key rotation procedure, break-glass procedure, production mode implications, and key storage recommendations for auditors and operators. * fix(ci): use matrix language as CodeQL job name Produces consistent check name 'Analyze (javascript-typescript)' matching branch protection requirements. * chore(config): harden staging profile and document environment setup Remove PRIVATE_KEY from staging.env, fix bridge destination to OP Sepolia, add key separation docs, fix env guard and drift guard for CI validation. * feat(frontend): replace dev dashboard with protocol info page Protocol info page with pre-production status, contract descriptions, and resource links. Providers updated to optimism/optimismSepolia. * chore(docs): cleanup and NatSpec improvements Fix README clone URL and naming, remove stale date from CONTRIBUTING.md, add eip712Domain NatSpec and no-pause design decision docs. * fix(contracts): document setVerifier interface check limitation Add @dev comment explaining code.length check rejects EOAs but not non-conforming contracts. * docs: add protocol philosophy to README Code is a rule. No DAO, no drama. Don't Trust, Verify. * fix(ci): add working-directory override to pre-checkout branch enforcement steps Fixes pre-checkout branch check failing with 'No such file or directory' in staging and production workflows. * fix(ops): enable post-deploy in rehearse-production-lock Enable MARK_RELEASE_RUN_POSTDEPLOY so activateProductionMode() is called during rehearsal. * fix(ops): export deployed verifier address to env before PostDeployMARKSetup Fixes VerifierRequiredWhenProofEnabled during staging rehearsal. * fix(ci): exclude Anvil default key from secrets drift guard Syncs Anvil key exclusion to dev. * test THREAT_MODEL.md: trust boundaries, role compromise impact, external dependencies, invariants, and explicit out-of-scope items. KNOWN_ISSUES.md: six accepted design decisions with rationale — attested verifier as ZK placeholder, no-pause design, setVerifier interface check limitation, counter overflow analysis, timestamp epoch manipulation, and transitive dep alerts. * fix(docs): correct two inaccurate invariants in THREAT_MODEL.md consumedIntents is set after proof validation, not before. Module balance invariant is per-operation, not absolute zero. * fix(contracts): move consumedIntents assignment before external call (CEI) Follows CEI pattern — marks intent consumed before external verifier call. No behaviour change for current view verifier. * chore(governance): set canary to 0 required approvals for solo maintainer Solo dev cannot self-approve. CI checks are the gate. Restore to 1 when second team member joins. * docs(contracts): add NatSpec to settleMint and settleBurn Documents pre-approval requirement for settleBurn. * fix(ops): wait for tx confirmation in staging rehearsal Add --slow to forge script broadcast so Foundry waits for each transaction receipt before the verify step runs. * fix(governance): set all branches to 0 required approvals Solo maintainer cannot approve own PRs. CI gates are the enforcement mechanism. Removes MAIN_REVIEW_COUNT/DEV_REVIEW_COUNT vars, adds approval count verification to verify-governance.sh. * fix(governance): restrict direct pushes to trade/maintainers team Restricts direct pushes on all branches to trade/maintainers team. Removes unused helper functions. verify-governance.sh now checks push restriction team slug. * fix(deps): update drizzle-orm dependabot ignore rule to 0.38.4 drizzle-orm@0.38.4 is transitive from @eth-optimism/super-cli. Updated ignore rule to match installed version. All four Dependabot alerts dismissed as tolerable risk. * feat(contracts): add Groth16SettlementVerifier Adds Groth16SettlementVerifier implementing IUTXOSettlementVerifier via swappable IGroth16Verifier. 12 unit tests passing. AttestedSettlementVerifier remains active production verifier. * feat(circuits): add UTXOSettlement circom circuit Adds UTXOSettlement circom circuit. Poseidon-based UTXO ownership proof. 602 constraints, 6 witness tests passing. * feat(contracts): add MARKPool ZK UTXO pool domain Adds MARKPool shielded RYLA transfer pool. 88 unit tests passing. * fix(contracts): rewrite MARKPool for MARK's 4-signal circuit Rewrites MARKPool from scratch for MARK's own UTXOSettlement circuit. UTXOVerifier.sol regenerated from MARK's own trusted setup. 84 unit tests passing. * fix(circuits): add range constraints and isMint burn path Range constraints on recipient/chainId/settlementModule/amount. isMint burn path in MARKPool. Trusted setup rerun. 84 tests passing. * feat(pool): add MARKPool ZK UTXO pool domain (#100) * feat(pool): add MARKPool ZK UTXO pool domain Introduces the full pool domain for private RYLA transfers: Contracts: - MARKPool: ZK UTXO pool with Merkle tree, fee policy, bridge-out/in, withdraw binding, AccessManaged access control - MARKWithdrawAdapter: EIP-712 signature-based withdrawal adapter - RYLACreditLedger: ICreditLedger adapter bridging MARKPool to RYLA mint/burn; restricted to pool caller only (onlyPool) - PoolFeePolicy, PoolPublicInputs, PoolValidation: pool support libraries - MARKPoolVerifier: Groth16 verifier generated from MARKPool circuit (13 public signals, pot15 trusted setup) Interfaces: ICreditLedger, IVerifier, IPoolBridge, IPoolNullifier Crypto: MerkleTree (Poseidon, depth-20), ProofUtils, PoseidonT3 Circuit: - circuits/mark/MARKPool.circom: MARK-native UTXO circuit (depth=20, 2-in/2-out, 13 public signals); renamed from prototype utxo.circom, domain constants documented as permanent, hardcoded fee policy removed - circuits/setup.mjs: trusted setup script (pot15) - circuits/test/MARKPool.test.mjs: 13 witness tests CI: circuits-ci.yml runs witness tests on every PR Tests: MARKPool.t.sol (22), MARKWithdrawAdapter.t.sol (9), RYLACreditLedger.t.sol (8) * fix(pool): fix PoolErrors, domain separators, remove dead code - PoolErrors.sol: rewrite to match Pool.sol, PoolValidation.sol, and MerkleTree.sol — adds 25 missing errors (build was broken), removes 18 errors only used by the old MARKPool prototype - MARKPool.sol: rename domain separator Pool.WithdrawBinding.v1 to MARKPool.WithdrawBinding.v1 (permanent, must be set before deploy) - MARKWithdrawAdapter.sol: rename domain separator WithdrawAdapter.Intent.v1 to MARKWithdrawAdapter.Intent.v1 - UTXOVerifier.sol: delete (built for old 4-signal circuit, wrong interface, superseded by MARKPoolVerifier.sol) - IUTXOVerifier.sol: delete (superseded by IVerifier.sol) - UTXOSettlement.circom: delete (superseded by MARKPool.circom) - Groth16SettlementVerifier.sol: update stale comment - KNOWN_ISSUES.md: add KI-7 (two-circuit architecture), KI-8 (pool domain access control model) - foundry.toml: via_ir = true for pool domain compilation * fix(pool): immutable naming, deploy script, docs, invariants, arch guard - MARKPool, MARKWithdrawAdapter: rename immutables to SCREAMING_SNAKE_CASE (assetLedger->ASSET_LEDGER, proofPool->PROOF_POOL) - MARKPool: remove _assetLedger from constructor; add setAssetLedger() one-time restricted setter to break circular deploy dependency with RYLACreditLedger - DeployMARKPool.s.sol: full deployment script for pool domain (AccessManager, MARKPool, RYLACreditLedger, MARKWithdrawAdapter) - MARKPool.sol: add withdrawal flow NatSpec (burn-to-claim model) - ARCHITECTURE.md: add pool/withdraw domains, dependency rules, and withdrawal flow section - MARKPoolInvariants.t.sol: 3 invariants (nullifiers never unspent, withdraw bindings immutable, root queue only grows) - architecture-guard.sh: add pool->settlement/bridge and withdraw->settlement/bridge isolation rules * fix(pool): fix deploy script role grant and ASSET_LEDGER null guard - DeployMARKPool.s.sol: grant POOL_ADMIN_ROLE to deployer during setup so setAssetLedger/setIntentSigner calls succeed when deployer != owner; revoke deployer role after setup completes - MARKPool._applyFee: revert InvalidAssetLedger if ASSET_LEDGER is not set and a non-zero fee is applied (prevents silent call to address(0)) * fix(ci): compile circuit before running witness tests circuits/build/ is gitignored so the WASM and witness_calculator.js are not in the repo. Add circom install and npm run build steps before npm test so CI compiles the circuit fresh on each run. * fix(ci): create build dir before circom compile * refactor(pool): pre-merge improvements - Rename immutables to SCREAMING_SNAKE_CASE: assetLedger->ASSET_LEDGER, proofPool->PROOF_POOL (MARKPool.sol, MARKWithdrawAdapter.sol) - MARKPool: remove _assetLedger from constructor, add setAssetLedger() one-time restricted setter to break circular deploy dependency with RYLACreditLedger - MARKPool: add withdrawal flow documentation to contract NatSpec - ARCHITECTURE.md: add pool/withdraw domains, dependency rules, and withdrawal flow explanation - DeployMARKPool.s.sol: deployment script for MARKPool, RYLACreditLedger, MARKWithdrawAdapter with AccessManager configuration - MARKPoolInvariants.t.sol: 3 invariants (nullifiers never unspent, withdraw bindings immutable, root queue only grows) - architecture-guard.sh: add pool and withdraw domain isolation rules * chore(pool): update circuits CI, setup, and pool errors - circuits-ci.yml: updated to run MARKPool witness tests - circuits/package.json: build/test scripts point to MARKPool.circom - circuits/setup.mjs: updated for MARKPool.circom trusted setup - circuits/test/MARKPool.test.mjs: cleaned up test file - contracts/KNOWN_ISSUES.md: updated KI-7 for current two-circuit state - contracts/src/pool/errors/PoolErrors.sol: add missing blank line * fix(pool): address CodeRabbit review findings - circuits-ci.yml: fix circom install permissions (use sudo mv to /usr/local/bin instead of direct write which fails on GH Actions) - PoolErrors.sol: add clarifying comment to FixedFeePolicy explaining it fires when minFee > 1 (not a fee-rate policy, a range guard) - MARKWithdrawAdapter.sol: document personal_sign intent on computeWithdrawIntentDigest (EIP-191 is intentional, not EIP-712) bridgeIn replay protection finding: already fixed in current code (processedBridgeMessages mapping + check at line 390) — stale finding. * fix(pool): address second round CodeRabbit findings - setup.mjs: use crypto.randomBytes for ceremony entropy (Date.now is predictable), add mkdirSync for build/, fix EJS template loading to use readFileSync instead of dynamic import with assert (unsupported in Node 20/22/24 ESM) - circuits-ci.yml: pin circom to v2.2.3 instead of latest, add version verification step - KNOWN_ISSUES.md: fix misleading 'settlement-specific verifier' wording — MARKPoolVerifier is a shared pool verifier, not settlement-specific - MARKPool.sol: fix NatSpec EIP-712 reference to EIP-191 (personal_sign) * feat(pool): add pool E2E test, fix RYLACreditLedger caller model RYLACreditLedger: - Separate credit (pool-only) and debit (adapter-only) callers - Add setAdapter() one-time setter to break circular deploy dependency (adapter constructor needs ledger, ledger needs adapter address) - Add AdapterAlreadySet error DeployMARKPool.s.sol: - Call ledger.setAdapter(adapter) after adapter deployment Tests: - RYLACreditLedger.t.sol: updated for new caller model, 11 tests - MARKWithdrawAdapter.t.sol: add setAdapter call in setUp - MARKPoolE2E.t.sol: full withdrawal flow E2E test (3 tests) - testFullWithdrawalFlow: mint RYLA -> transactWithWithdrawBinding -> withdrawWithSig -> verify RYLA burned, ETH received - testNullifierReplayRejected - testBindingMismatchRejected 134/134 tests pass * feat(pool): add ReleasePool.s.sol orchestrator and pool env vars - ReleasePool.s.sol: release orchestrator for pool stack following the same pattern as ReleaseMARK.s.sol — preflight checks, deploy via DeployMARKPool, post-deploy verification (wiring checks + RYLA roles), JSON artifact write - .env.example: add pool stack env vars (MARK_POOL_VERIFIER, MARK_POOL_OWNER, MARK_POOL_INTENT_SIGNER, release flags, artifact path, post-deploy verify addresses) * fix(pool): security fixes and dead code removal RYLACreditLedger: - Add OWNER immutable (set to msg.sender in constructor) - Restrict setAdapter to OWNER to prevent front-running between deployment and the setAdapter call in the release script - Add testSetAdapterRevertsForNonOwner test - Add clarifying NatSpec to totalCreditsOutstanding explaining it tracks only flows through this ledger, not total RYLA supply MARKWithdrawAdapter: - Move ETH transfer before ASSET_LEDGER.debit — if ETH transfer fails, RYLA is no longer burned (was a loss-of-funds bug) MARKPool: - Remove dead _seedRoot function (defined but never called) - Add NatSpec to computePublicInputsWithWithdraw clarifying chainId vs dstChainId semantics * fix(test): fix nullifier replay test to use fresh signatures testNullifierReplayRejected was reusing signatures computed for nonce N in the second withdrawWithSig call with nonce N+1, causing a NonceMismatch revert instead of exercising nullifier replay protection. Now recomputes the intent hash and signs with the updated nonce so the revert is caused by NullifierAlreadyClaimed as intended. * fix(pool): guard totalCreditsOutstanding against underflow * feat(pool): add pool release CI check and deploy script tests contracts-ci.yml: - Add pool release dry-run and execute smoke steps to the contracts-release-check job, reusing the Anvil instance and RYLA token deployed by the settlement release step - Assert pool release artifact schema (pool, ledger, adapter addresses) MARKPoolDeployScripts.t.sol: - testDeployMARKPoolWiresAllContracts: verifies all contract wiring (pool<->ledger, ledger<->adapter, RYLA roles) - testDeployMARKPoolSetsIntentSignerWhenProvided: verifies intent signer is configured when MARK_POOL_INTENT_SIGNER is set - testDeployMARKPoolRevertsWhenMissingTokenAdmin: verifies preflight check rejects deployer without RYLA admin role 138/138 tests pass * fix(pool): address final CodeRabbit findings - contracts-ci.yml: remove '|| true' from pool release dry-run step; use the deployed settlement module address as verifier (a real contract) so the preflight code.length check passes without masking failures - RYLACreditLedger.sol: fix NatSpec on totalCreditsOutstanding to accurately describe accounting scope — _totalBurned can exceed _totalMinted if RYLA is burned via other paths (e.g. settlement module) * fix(ci): fix pool release CI failure and address CodeRabbit finding contracts-ci.yml: - Add --skip-simulation to pool release broadcast — PoseidonT3 (55,856 bytes) exceeds EIP-170 limit and cannot be deployed without refactoring to a linked library; --skip-simulation tests script orchestration only - Fix jq assertion to use regex validation instead of zero-address check, rejecting null values and validating hex address format KNOWN_ISSUES.md: - Add KI-8 documenting PoseidonT3 contract size issue and required fix before mainnet (deploy as standalone contract, call via interface) * fix(ci): remove pool execute smoke, fix jq assertion, fix KI-7 wording contracts-ci.yml: - Remove pool release execute smoke step — MARKPool (24,841 bytes) and PoseidonT3 (55,856 bytes) exceed EIP-170 limit and cannot be broadcast to Anvil; pool deploy requires PoseidonT3 refactor (KI-8) first - Keep pool release dry-run only (validates script logic and preflight) - Remove the now-unused artifact assertion step KNOWN_ISSUES.md: - Fix KI-7: both pool and settlement systems use the same MARKPool circuit — remove implication of distinct circuit designs * fix(pool): add code.length checks to RYLACreditLedger constructor and setAdapter Prevents EOAs from being set as TOKEN, POOL, or ADAPTER. Adds InvalidContract error. 3 new tests cover the EOA rejection cases. setUp uses vm.etch to give mock addresses contract bytecode. * fix(contracts): harden settlement verifier flow and CI reliability * fix(review): address open CI and pool verifier feedback * refactor(pool): rename min fee guard error for clarity * fix(pool,settlement): replace require strings and wrong errors with custom errors PoolFeePolicy: - Replace require(maxFeeBurnBps != 0, string) and require(feeBurnBps <= maxFeeBurnBps, string) with custom error FeePolicyInvalidBps() — consistent with codebase style, lower gas Groth16SettlementVerifier: - Replace ZeroAddress() with VerifierNotAContract() for verifierContract code.length check - Replace ZeroAddress() with SettlementModuleNotAContract() for settlementModule code.length check - ZeroAddress was semantically wrong for non-zero addresses that have no code * ci: trigger fresh CI run * docs(pool): correct KI-8 — MARKPool itself is over EIP-170 size limit Investigation: MARKPool is 24,960 bytes (over 24,576 limit) even without PoseidonT3 inlining. via_ir=true already prevents PoseidonT3 from being inlined. The fix requires splitting MARKPool into smaller contracts, not just extracting PoseidonT3 as a standalone contract. Both are required. * fix(pool): reduce MARKPool below EIP-170 size limit (24200 < 24576 bytes) Size reductions (24961 -> 24200 bytes, -761 bytes): - Remove redundant verifierAddr.code.length check in _verifyAndConsume (already validated in setVerifier, cannot change after deployment) - Remove redundant tail != rootQueueTail guard in _insertCommitmentsValidated (always true after inserting 2 commitments) - Inline _requireCommitmentsValid wrapper (single-line delegation) - Inline _insertCommitments wrapper (only called from bridgeIn) - Remove computePublicInputs and computePublicInputsWithWithdraw public view functions from MARKPool — _buildPublicInputs now calls PoolPublicInputs.build directly; off-chain callers use PoolPublicInputs Bug fixes: - PoolValidation: move NullifierDuplicate check before the loop so duplicate nullifiers get the precise error, not NullifierUsed - MARKPool.pause(): document that unpause() does NOT auto-restore withdrawals (intentional asymmetry, requires explicit unpauseWithdrawals) * fix: address CodeRabbit findings (circuits, Makefile, architecture-guard) circuits/test/MARKPool.test.mjs: - Remove unused buildMerklePath helper (tests use buildTwoLeafRoot) circuits/setup.mjs: - Add r1cs existence check before trusted setup with clear error message contracts/Makefile: - Restore test-core to exclude invariant tests (--no-match-path) so ci-fast remains fast as documented contracts/script/ci/architecture-guard.sh: - Tighten all four import regexes to handle optional leading whitespace and any number of ../ segments (prevents bypass via indented imports or deeper relative paths) * fix: address remaining CodeRabbit findings contracts/src/pool/MARKPool.sol: - setVerifier: add code.length check (consistent with constructor) circuits/test/MARKPool.test.mjs: - expectFail: only treat constraint/assertion failures as PASS; rethrow other errors so regressions surface contracts/KNOWN_ISSUES.md: - KI-7: separate design capability from configuration state for settlement system wording * fix(circuits): lowercase error message comparison in expectFail * docs(deployment): add Groth16SettlementVerifier wiring step (Step 18) Documents the two post-deploy calls required to activate ZK-based settlement: setSettlementModule and setVerifierContract on Groth16SettlementVerifier, then setVerifier on MARKSettlementModule. AttestedSettlementVerifier remains the fallback until wiring is complete. * fix(settlement): return false on malformed proof in Groth16SettlementVerifier (#101) abi.decode reverts on malformed/short proof bytes, which propagated through MARKSettlementModule as a raw error instead of VerificationFailed. Fix: check proof.length == 672 before decoding (fixed ABI encoding size: uint256[2]+uint256[2][2]+uint256[2]+uint256[13] = 64+128+64+416 = 672). Malformed proofs now return false cleanly. Tests: testVerifySettlementReturnsFalseForMalformedProof, testVerifySettlementReturnsFalseForEmptyProof * fix(ci): exclude integration tests from test-core target (#102) test-core was running integration tests (which require supersim on port 9545) because --no-match-path on the command line overrides foundry.toml's no_match_path setting rather than adding to it. Use brace glob to exclude both invariant and integration tests. * fix(test): remove unverifiable cross-chain assertion from integration test (#103) testBridgeToTransfersTokensCrossChain switched to fork B and checked the recipient balance, but Foundry fork tests cannot simulate supersim's async message relay — the contract simply doesn't exist on the other fork. Fix: assert only the source-chain burn (which is fully verifiable in a fork test). Add a NatSpec note explaining the relay limitation. * docs(pool): correct KI-8 — PoseidonT3 inlined via via_ir, MARKPool deployable (#104) * docs(pool): correct KI-8 — PoseidonT3 is inlined via via_ir, MARKPool is deployable via_ir=true causes the compiler to inline PoseidonT3 into MARKPool rather than deploying it as a linked library. MARKPool has no link references and is 24,298 bytes (278 bytes under EIP-170). KI-8 was based on an earlier state where MARKPool exceeded the limit. Updated KI-8 to reflect accurate current state and note the tight margin. * refactor(crypto): use >>= 1 instead of /= 2 in MerkleTree insert * security: harden pool domain before testnet (#105) * security: harden pool domain before testnet - Add pool/withdraw/Groth16 contracts to slither-core scope - Document all slither exclusion rationale in Makefile - RYLACreditLedger: add Credit/Debit events, move before external calls (CEI) - MARKWithdrawAdapter: add test for recipient zero-check (existing check, missing test) - THREAT_MODEL.md: add pool stack overview, trust boundaries, role compromise impact, and 3 new invariants (nullifier replay, withdraw binding, debit approval) * fix(ci): use per-contract slither exclusions instead of global CodeRabbit correctly noted that global exclusions could suppress actionable findings in newly added contracts. Refactored slither-core to apply only the relevant exclusions per contract. Also added arbitrary-send-erc20 to MARKSettlementModule and RYLACreditLedger (both use safeTransferFrom with prior approval — not arbitrary). * fix(ci): add set -e to slither-core, fix preflight to use python3 -m slither Without set -e, a failing early slither invocation would be masked if the final command succeeds. Also align the preflight check with the actual invocation (python3 -m slither, not command -v slither). * ci: fix 4 workflow issues pre-testnet (#106) * ci: fix 4 workflow issues pre-testnet 1. Sync _reusable-contracts-slither.yml with Makefile - Delegate to 'make slither-core' (single source of truth) - Now covers all 8 contracts with per-contract exclusions - Previously only scanned 4 settlement contracts with global exclusions 2. Enable pool execute smoke in contracts-ci.yml - KI-8 resolved: via_ir inlines PoseidonT3, MARKPool is 24,298 bytes - Pool broadcast to Anvil now works; remove stale blocker comment 3. Fix integration test readiness check - Wait on ports 9545/9546 (actual RPC ports) not 8420 (admin port) - Use nc loop consistent with anvil readiness pattern 4. Pin foundry-rs/foundry-toolchain to v1.8.0 commit SHA - Floating @v1 could silently break on Foundry breaking changes - Pinned: c7450ba673e133f5ee30098b3b54f444d3a2ca2d (v1.8.0) * fix(ci): remove foundry version input from reusable slither workflow The version input was passed as 'v1.8.0' to the action's 'version' input which expects a Foundry binary tag (e.g. 'stable', 'nightly'), not the action version. This caused foundryup to fail extracting the tar archive. Use the action's default Foundry version instead. * fix(ci): revert pool execute smoke — Foundry rejects PoseidonT3 artifact size forge create/broadcast checks all library artifacts for EIP-170 compliance. PoseidonT3 is 55,856 bytes as a standalone artifact even though via_ir inlines it into MARKPool at compile time. The broadcast is blocked before deployment. Keep dry-run only. Update KI-8 with the precise diagnosis. * fix(pool): resolve PoseidonT3 deployment blocker via external interface (#107) PoseidonT3 is a Solidity library with a public function — it gets deployed as a separate linked contract (55,856 bytes) which exceeds EIP-170 (24,576). This blocked all pool deployments. Fix: replace the library call with an external interface (IPoseidonT3). MerkleTree now stores the Poseidon contract address in the Tree struct and calls it via DELEGATECALL-free external call. MARKPool constructor accepts a _poseidon address parameter. Default deployment address: 0xB43122Ecb241DD50062641f089876679fd06599a This is Semaphore's PoseidonT3 (PSE/Ethereum Foundation), deployed at the same address on all EVM networks via CREATE2. Verified compatible with our implementation: hash([0,0]) and hash([1,2]) produce identical outputs. MARKPool now has zero link references and is fully self-contained. MARKPool size: 24,231 bytes (345 bytes margin under EIP-170). Tests: deployCode('PoseidonT3.sol:PoseidonT3') in test setUp bypasses EIP-170 (Foundry test runner does not enforce the limit). * chore(circuits): remove stale UTXOSettlement artifacts (#108) * chore(circuits): remove stale UTXOSettlement artifacts UTXOSettlement circuit is superseded by MARKPool.circom. Remove the stale test file and old verification key artifact. The utxo/ source and build/ artifacts are already gitignored. * ci: trigger Release Gate Container for circuits-only PRs Add circuits/** to path filter so the required check runs and passes when only circuit files change (no contracts affected). * ci: add circuits/** to push paths for consistency * ci: remove path filter from release gate pull_request trigger * ci: add circuits/** to CodeQL path filter to unblock circuits-only PRs * fix: address codebase review findings (#109) Bug: RYLACreditLedger.debit() — move _totalBurned update before safeTransferFrom to follow CEI pattern. Previously the state update happened after the external call, creating a reentrancy window where _totalBurned was not yet incremented during the transfer callback. Docs: KNOWN_ISSUES.md KI-8 — update stale size figures and description. MARKPool is now 24,231 bytes (345 bytes margin). PoseidonT3 is no longer inlined via via_ir; MerkleTree calls it via IPoseidonT3 interface at 0xB43122... (Semaphore, same address on all EVM networks). Tests: add testConstructorRevertsOnZeroPoseidon and testConstructorRevertsOnEOAPoseidon to MARKPool.t.sol — the _poseidon constructor parameter added in PR #107 had no test coverage. * ci: pin action-shellcheck to commit SHA (#110) * ci: pin action-shellcheck to commit SHA ludeeus/action-shellcheck@2.0.0 was pinned by version tag only. Tags are mutable — a compromised tag could point to malicious code. Pin to the immutable commit SHA (00cae50) for supply chain safety. * ci: trigger CodeQL for all .github/workflows/** changes * chore(deps): bump actions/dependency-review-action from 4 to 5 (#90) Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 4 to 5. - [Release notes](https://github.com/actions/dependency-review-action/releases) - [Commits](https://github.com/actions/dependency-review-action/compare/v4...v5) --- updated-dependencies: - dependency-name: actions/dependency-review-action dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * chore(deps): bump the frontend-minor-patch group across 1 directory with 21 updates (#91) Bumps the frontend-minor-patch group with 6 updates in the / directory: | Package | From | To | | --- | --- | --- | | [@tailwindcss/vite](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/@tailwindcss-vite) | `4.2.4` | `4.3.0` | | [tailwind-merge](https://github.com/dcastil/tailwind-merge) | `3.5.0` | `3.6.0` | | [tailwindcss](https://github.com/tailwindlabs/tailwindcss/tree/HEAD/packages/tailwindcss) | `4.2.4` | `4.3.0` | | [baseline-browser-mapping](https://github.com/web-platform-dx/baseline-browser-mapping) | `2.10.27` | `2.10.29` | | [electron-to-chromium](https://github.com/Kilian/electron-to-chromium) | `1.5.352` | `1.5.353` | | [get-east-asian-width](https://github.com/sindresorhus/get-east-asian-width) | `1.5.0` | `1.6.0` | Updates `@tailwindcss/vite` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/packages/@tailwindcss-vite) Updates `tailwind-merge` from 3.5.0 to 3.6.0 - [Release notes](https://github.com/dcastil/tailwind-merge/releases) - [Commits](https://github.com/dcastil/tailwind-merge/compare/v3.5.0...v3.6.0) Updates `tailwindcss` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/packages/tailwindcss) Updates `@tailwindcss/node` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/packages/@tailwindcss-node) Updates `@tailwindcss/oxide-android-arm64` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/android-arm64) Updates `@tailwindcss/oxide-darwin-arm64` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/darwin-arm64) Updates `@tailwindcss/oxide-darwin-x64` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/darwin-x64) Updates `@tailwindcss/oxide-freebsd-x64` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/freebsd-x64) Updates `@tailwindcss/oxide-linux-arm-gnueabihf` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-arm-gnueabihf) Updates `@tailwindcss/oxide-linux-arm64-gnu` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-arm64-gnu) Updates `@tailwindcss/oxide-linux-arm64-musl` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-arm64-musl) Updates `@tailwindcss/oxide-linux-x64-gnu` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-x64-gnu) Updates `@tailwindcss/oxide-linux-x64-musl` from 4.2.4 to 4.3.0 - [Release notes](https://github.com/tailwindlabs/tailwindcss/releases) - [Changelog](https://github.com/tailwindlabs/tailwindcss/blob/main/CHANGELOG.md) - [Commits](https://github.com/tailwindlabs/tailwindcss/commits/v4.3.0/crates/node/npm/linux-x64-musl) Updates `@tailwindcss/oxide-wasm32-wasi` from 4.2.4 to 4.3.0 - [Release not…
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
1 participant
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.
Summary
Introduces the full pool domain for private RYLA transfers using ZK UTXO proofs with Merkle tree membership.
Changes
Contracts
Circuit
Fixes and cleanup
CI
Scope
contracts, circuits, workflows
Verification
Risk
Medium. New pool domain is additive — no changes to existing settlement or bridge contracts. RYLACreditLedger access control fix (onlyPool) is a security improvement. Domain separator rename is permanent and must land before any pool deployment.
Summary by CodeRabbit
New Features
Libraries / Interfaces
Docs / Ops
Chores