feat(broker): select the assessor per batch from the on-chain router registry#2040
Draft
jonastheis wants to merge 36 commits into
Draft
feat(broker): select the assessor per batch from the on-chain router registry#2040jonastheis wants to merge 36 commits into
jonastheis wants to merge 36 commits into
Conversation
This reverts commit e1c5da1 to re-introduce `OnChainAssessor` and its unit tests/benches on top of `jonas/router-decoupling`. The parent branch keeps the OnChain code out of its scope; this branch re-adds it for review in its own PR.
…ack bytes for ClaimDigestMatch Two related fixes to the on-chain assessor path: * Predicate.eval(imageId, journal) and OnChainAssessor were hashing sha256(abi.encode(journal)) while the off-chain R0 STARK guest, BoundlessMarketCallback, and the broker's claim-digest computation use sha256(journal) over the raw bytes. The on-chain side never matched a real fill. Both call sites now use the canonical convention. * OnChainAssessor did not reconstruct the claim digest from (imageId, journal) when the predicate was ClaimDigestMatch and the prover attached an ImageIdAndJournal fulfillment payload (typically to feed a callback). The off-chain assessor guest enforces this binding; the on-chain adapter now matches it. Without the guard, a malicious prover could submit a valid seal for the requested claim digest but attach (imageId, journal) bytes that don't match, and the market would dispatch a callback with unproven data.
…geHashUtils
* `FULFILLMENT_BATCH_AUTH_TYPE` and `FULFILLMENT_BATCH_AUTH_TYPEHASH` go
from `internal constant` to `public constant`. Brokers, wallets, and
tests can now read the canonical typehash directly from the deployed
adapter instead of redeclaring it, eliminating a class of
silent-drift bugs where a renamed type string would invalidate every
pre-computed seal in flight.
* The EIP-712 digest is now built via
`MessageHashUtils.toTypedDataHash(DOMAIN_SEPARATOR, structHash)`
instead of inlining `keccak256(abi.encodePacked("\\x19\\x01", ...))`.
Same bytes, but the magic prefix and assembly live in OZ's audited
helper rather than this contract.
Registers `OnChainAssessor` as a third assessor entry in the `BoundlessMarketTest` router setup and adds matching broker-side fixtures: * `createFulfillmentBatchOnChain` / `createFillsAndSubmitRootOnChain` produce a `FulfillmentBatch` whose `assessorSeal` is a real EIP-712 ECDSA signature by the prover wallet over the batch's `(prover, requestDigests, claimDigests)` tuple — the on-chain equivalent of what the R0 STARK helper produces with a journal commitment. The two variants differ only in whether the per-fill seal is a `NullVerifier` placeholder or a real set-builder inclusion proof (needed for callback tests). * `_buildOnChainAssessorSeal` sources the typehash and EIP-712 digest from the deployed adapter (`FULFILLMENT_BATCH_AUTH_TYPEHASH`, `DOMAIN_SEPARATOR`) and `MessageHashUtils`, so the helper stays in lock-step with what the contract verifies. `BoundlessMarketOnChainAssessorTest` exercises the production fulfill flow through that adapter end-to-end: locked-request happy path, batch fulfill, the ClaimDigestMatch + ImageIdAndJournal callback scenarios (both matching and mismatched journal bytes — the latter pins the adapter's reconstruction guard from blocking unproven callback data), and a mixed-adapter batch where one fulfillment goes through `OnChainAssessor` and another through `R0BoundlessAssessorAdapter` in the same transaction. Snapshot deltas come from gas changes introduced by the upstream `MessageHashUtils.toTypedDataHash` swap.
Replaces the prior 6 happy-path / single-revert tests with 26 unit tests that pin the adapter's behavior end-to-end against the same contract a broker drives in production. Self-contained: inherits from `Test` directly, deploys a single `OnChainAssessor` in `setUp`, calls it directly (no router, no harness wrapper, no shared bench ecosystem). Fixture builders are inlined below the test methods so the file reads top-to-bottom. Coverage: * Single-fill + multi-fill happy paths for `DigestMatch`, `ClaimDigestMatch`, `PrefixMatch`, plus a mixed-predicate batch that exercises the per-fill switch. * `ClaimDigestMatch + ImageIdAndJournal` reconstruction guard, both the matching (passes) and mismatched (`ClaimDigestMismatch`) paths — the latter pins the divergence guard added in `dd2d7dd1`. * Predicate-failure shape per predicate kind (wrong journal, wrong imageId, prefix that doesn't match). * `MissingFulfillmentData` for predicates that require a journal. * Length-mismatch guards on `fills` / `requestDigests`. * Malformed-seal lengths (selector-only, one byte short, one byte long — the adapter requires exactly 4 + 65 bytes). * Tamper detection on `requestDigest`, `claimDigest`, `fulfillmentData`, and `prover` after the prover has signed. * ERC-165 conformance + a regression test that asserts the contract's `FULFILLMENT_BATCH_AUTH_TYPEHASH` still matches the literal type string a wallet would sign against — so a silent rename trips loudly instead of invalidating every pre-computed seal in flight. * Domain-separator binding. Seal construction uses the adapter's public typehash plus `MessageHashUtils.toTypedDataHash`, so the helper stays in lock-step with what the contract verifies.
…ention `BenchBase._makeFill` was producing claim digests with `sha256(abi.encode(journal))`, which diverged from the off-chain R0 STARK guest, `BoundlessMarketCallback`, and the broker's claim-digest computation — all of which use `sha256(journal)` over the raw bytes. The bench harness now uses the canonical hash, matching the on-chain `Predicate.eval` / `OnChainAssessor` fix from `dd2d7dd1`. Also drops the stale comment on `AdapterBench.test_bench_journalSize` that referenced the old hashing pattern.
Move the (imageId, journal) reconstruction check ahead of the predicate dispatch so it runs once per fill whenever an ImageIdAndJournal payload is attached, instead of being duplicated across the ClaimDigestMatch and DigestMatch/PrefixMatch branches. Behavior-preserving for valid fills; for invalid fills the selector surfaced when both checks would fail is now ClaimDigestMismatch rather than PredicateFailed. Tests: - digestMatch_wrongJournal / wrongImageId: expect ClaimDigestMismatch to match the new ordering. - prefixMatch_journalDoesNotStartWithPrefix: update fill.claimDigest so the reconstruction guard passes, isolating the prefix-eval branch as the failure path (the only test exercising PredicateFailed for non-ClaimDigestMatch predicates). - tamper_fulfillmentData_postSigning: rework with PrefixMatch and a prefix-preserving new journal so predicate + reconstruction both pass and only the per-batch signature catches the mutation.
…ing merge The merge of router-decoupling was textually clean but left one semantic break: ProofDelivered gained a requestDigest arg (4 total), while the assessor branch's added test still emitted it with 3, failing the build. - Pass expectedRequestDigest in testFulfillLockedRequest_OnChainAssessor - Regenerate gas snapshots: drop stale :v2 keys (test code already dropped them) and pick up the -9KB impl bytecode / per-call gas shift from the router decoupling - Regenerate the Predicate.sol artifact (sha256(journal), no abi.encode) and the assessor-guest Cargo.lock (transitive serde_bytes)
…as/market-legacy-fallback
…apshot The broker now mirrors the on-chain BoundlessRouter model: a request's signed verifier selector resolves to a verifier class, the class's requiredAssessorClass names the assessor class, and the broker picks the highest-priority candidate assessor registered there (preferring the native OnChainAssessor signature over proving the R0 STARK guest). - SDK: RouterRegistry snapshots the router (entries/classes/ defaultClassId) once at startup and answers resolution queries as pure in-memory lookups; canonical ONCHAIN_ASSESSOR_SELECTOR / R0_ASSESSOR_SELECTOR constants; the DOMAIN_SEPARATOR probe and the market.assessor_selector config knob are removed. - risc0-backend: a mandatory ResolvedRouter (registry + derived policy) replaces the per-backend startup assessor strategy; the assessor is derived per batch from its orders' verifier class, the assessor guest is skipped when the on-chain assessor is selected, and supported_selectors only advertises what the backend can actually seal. Construction goes through Risc0Backend::from_deployment, which loads the registry even in listen-only mode. - broker: batches stay single assessor class — the open batch locks to one assessor group (Backend::assessor_group) and other-group orders are requeued for a later batch; orders with no resolvable assessor are failed loudly instead of silently deferred. - tests: boundless_test_utils::market::test_router_registry provides an in-memory fixture mirroring the deployed test router, so unit tests exercise the real resolution logic over canned data.
jonastheis
requested review from
a team,
Wollac,
bobbobbio,
capossele,
nahoc,
unizarr,
willemolding and
zeroecco
as code owners
…R0 guest fallback - e2e_r0_guest_assessor_fallback: tombstones the on-chain assessor entry before the broker starts, so the assessor priority falls back to the R0 STARK guest — proving the guest, set-builder aggregation, and the set-inclusion assessor seal end to end through the router. Adds a removeEntry binding to the BoundlessRouter test interface for it. - submitted_assessor_selector: decodes the fulfillment transaction's calldata and returns the 4-byte selector framing the batch's assessorSeal. Every e2e test now asserts which assessor adapter the router dispatched to: the on-chain assessor (0x00000022) everywhere, and the R0 guest (0x00000024) in the fallback test.
…al token - Deploy.s.sol: probe the configured collateral token with a balanceOf staticcall before reusing it. Deploy runs write the deployed address back into deployment.toml, so on a fresh chain the configured address is stale and - because deploys are deterministic - usually occupied by a different contract; code existence alone made the deploy skip HitPoints and produce a market whose collateral could be neither minted nor deposited. - localnet-deploy.sh: make the container-specific paths and anvil URL env-overridable and the env generation portable to BSD sed, so the script also runs directly on the host (e.g. arm64 machines where the amd64-only builder-base image blocks the deployer container). - localnet skill: raise the suggested offer prices; the batched-ABI lock+fulfill gas estimate (~620k gas) exceeds the old 0.0004 ETH max price, which makes the broker skip the order outright.
The broker now selects the assessor per batch from the router registry, so the config knob no longer exists. The bench harness registers both assessor adapters via the test fixtures, and the broker resolves the selector itself.
…e poisoning - Manage.s.sol: reference the upgrade-safety baseline by file-qualified name. The legacy fallback source (BoundlessMarketLegacy.sol) also declares a contract named BoundlessMarket, so the bare name is ambiguous in any build that includes contracts/src/legacy/. - contracts.yml: split the main build-info cache into explicit restore/save. The unified actions/cache saves in its post-job hook, by which time the workspace holds the PR branch — caching the PR's build-info under the main key. Later runs then validate upgrades against the wrong reference (in the degenerate case against the PR itself, making the storage-layout check vacuous). The key bump escapes the already-poisoned immutable entry.
…ification A real Groth16 verification costs ~250k gas (blake3-groth16 more), but the router deploy capped verifier-class entries at 50k (100k in the test harness). The cap bounds what a runaway adapter can burn per fill, not the expected cost — dev-mode mock verifiers stay far below it, which is why every dev run passed while the real-proving nightly reverted every fulfillment with VerifierFailed. Validated against a Bento cluster: the composition and blake3-groth16 examples (the failing nightly cases) now fulfill real groth16 and blake3-groth16 fills through the router.
…env in test brokers - smart-contract-requestor transitively imports BoundlessRouter, which needs via-ir to compile (stack too deep in legacy codegen); mirror the root foundry.toml compiler settings. - BrokerBuilder hardcoded the prover endpoints to None; read BENTO_API_URL / BONSAI_API_URL / BONSAI_API_KEY from the environment so real-proving runs can offload to a cluster instead of proving via the local r0vm. Dev-mode runs are unaffected.
…g replaced The legacy ABI serves imageInfo() from proxy storage, which a fresh proxy leaves empty, and the legacy initializer can never run on it (the new initialize consumes the shared Initializable version) — so old brokers, which fetch the assessor guest from that URL, fail against a new proxy. Both deploy flows now read the URL from the market being replaced (deployment.toml's boundless-market; LEGACY_MARKET overrides the address, LEGACY_ASSESSOR_GUEST_URL the URL) and set it through the fallback via the admin-only setImageUrl in the same run. The upgrade flow captures the proxy's own URL pre-upgrade and guarantees imageInfo() still serves it afterwards, which also exercises the fallback wiring of the new impl.
…roxies UpgradeBoundlessMarket resolved the delegate-call target with market.LEGACY_IMPL(), which only exists on the router-aware impl — and vm.envOr evaluates its default eagerly, so upgrading a proxy still on the legacy impl (the production in-place migration) reverted even with BOUNDLESS_LEGACY_IMPL set. The target is now resolved by probing the proxy: a router-aware proxy preserves its LEGACY_IMPL() so the fallback chain stays flat at one audited hop, and a legacy proxy takes the implementation being replaced. RollbackBoundlessMarket logged ROUTER() unconditionally after the swap, which made rolling back to a legacy impl — the emergency exit of that same migration — fail in simulation. The log now tolerates a legacy target. Validated on anvil with a proxy running the legacy impl natively: upgrade picks the replaced impl as fallback target and preserves the imageInfo() URL, rollback lands back on the legacy impl, and a router-to-router upgrade still carries LEGACY_IMPL() forward.
A router class is now a proof-type version family: R0SetInclusion (0xAA000001, chain default), R0Groth16 (0xAA000003), R0Groth16Blake3 (0xAA000004), each requiring the R0Assessor class (0xAA000002). Requestors can sign a class id to mean "any version of this proof type". DeployRouter shrinks to deploying the proxy. All class and entry registration lives in Manage.Router.s.sol:BootstrapRouter, which configures a fresh router in one idempotent run: everything already registered is skipped, canonical groth16/blake3 selectors are resolved against the upstream router and skipped when absent (localnet dev verifiers use dynamic selectors), and the set-inclusion selector is read from the set verifier's SELECTOR(). Incremental additions are a constant bump in the new RouterConfig library plus a bootstrap re-run, replacing the per-entry register scripts; RemoveEntry stays for explicit tombstoning and TransferRouterAdmin hands ADMIN_ROLE to a Safe/timelock after bring-up. The Rust class-id constants mirror RouterConfig and the test fixture and harness deploy the same per-type layout; RouterPolicy resolves it unchanged.
Signing the blake3-groth16 class id means "any version of this proof type", which is incoherent for blake3: its claim-digest construction folds the verifier's control root into the digest, so the predicate binds one specific verifier version. RequirementsLayer now rejects it at build time with a message pointing to the entry selector. This is specific to blake3's digest construction, not to ClaimDigestMatch in general — a standard RISC Zero claim digest commits to the execution claim (image id, journal, exit code), not the verifier version, so other proof types stay version-agnostic under a class id and are unaffected. Leaves a TODO to eventually validate signed selectors against a live on-chain BoundlessRouter snapshot, which would generalize this single hardcoded rule into registry-backed guidance (unknown selector, class permissionlessness, etc.).
…ests Add a repeatable --selectors flag (comma lists too): submit one request per value — a router class id, an entry selector, or 0x00000000 for the chain default — then await all fulfillments together. Blake3 selectors commit to a 32-byte journal and require building with --features blake3-groth16. Defaults to a single chain-default request, matching the prior behavior. Supports exercising the router validation matrix by signing specific classes or entry selectors.
DeployRouter and the Manage.Router scripts now read their inputs from the CHAIN_KEY section of deployment.toml — router proxy, application-verifier (upstream R0 router), set-verifier, assessor-image-id, admin — each still overridable by the env var of the same name. Matches how the market deploy scripts already resolve config, so a chain is configured in one place rather than through a long env-var list, and DeployRouter records the deployed proxy back into the section. - manage: quote FORGE_SCRIPT_FLAGS as a bash array so a multi-flag value is not word-split - localnet-deploy.sh: pass CHAIN_KEY through to the router scripts so they resolve the anvil section
The class-signed-blake3 test module sat before impl RequirementsLayer, tripping clippy::items_after_test_module under -Dwarnings (CI rust-lint). Move it to the end, matching the convention in the sibling modules.
…licit, share the registry
supported_selectors() sourced a hardcoded list that could only hold
compile-time constants, so the guest-version-derived set-inclusion entry
selector was absent — the broker skipped any request pinning the exact
version of the seal it produces by default. RouterPolicy now owns the
supported-set computation: producible entry selectors (assessor-filtered)
+ supported class ids + the chain-default sentinel (derived from the
default class being supported, not bolted on). Every producible pin is
now accepted.
A verifier class is a proof-type version family and may hold several
producible entries. Signing the class id ("any version") resolves to the
backend's most-preferred entry deterministically: producible_entries is a
descending-preference list and the first match per class wins, replacing
the previous incidental last-wins-by-Vec-order. Dev fakes are listed
first since they are what the broker produces under RISC0_DEV_MODE.
Covered by new multi-entry-class unit tests.
RouterPolicy holds the registry behind an Arc: the snapshot is global
chain state, so policy clones are refcount bumps rather than deep copies,
and one snapshot can be shared across backends once there is more than one.
Blake3-groth16 fulfillments use a ClaimDigestMatch predicate and carry no journal, so journal() is None — the unconditional unwrap panicked after the request had already fulfilled. Print a "(no journal in fulfillment)" line instead.
jonastheis
force-pushed
the
jonas/adopt-router-in-broker
branch
from
4c430f3 to
8b05c72
Compare
…ient compat Redefine ProofDelivered to carry LegacyFulfillment — a tuple byte-identical to the pre-router Fulfillment (id + requestDigest inline) — so the event keeps its original topic0 and stays decodable by clients that haven't upgraded their SDK. The market reconstructs the legacy shape at emit time from the request identity plus the current slimmed Fulfillment. SDK fulfillment queries (get_request_fulfillment, wait_for_request_fulfillment) return LegacyFulfillment directly. Indexer ProofDelivered handling reverts to consuming the legacy payload (id/requestDigest read off the event again), leaving log_processors.rs identical to main.
jonastheis
force-pushed
the
jonas/adopt-router-in-broker
branch
from
8b05c72 to
d49042c
Compare
This reverts commit c812bde.
A request may sign the blake3-groth16 router class id (R0_GROTH16_BLAKE3_CLASS_ID) rather than the concrete entry selector; the router resolves the class to that entry on-chain. Recognize the class id in is_blake3_groth16_selector so request building and predicate evaluation give the class the same 32-byte journal + ClaimDigestMatch construction as the entry selector.
…e info _printGnosisSafeInfo emitted upgradeTo(address) when initializerData was empty, but OpenZeppelin v5 removed the bare upgradeTo — the UUPS proxy only implements upgradeToAndCall(address,bytes), so that Safe tx reverts. Always emit upgradeToAndCall (empty initializerData = no init call).
…stered verifier The set-inclusion and R0 STARK assessor entries wrapped the bare set-verifier from deployment.toml, while groth16/blake3 wrapped whatever the upstream router resolves via getVerifier. On chains where the router fronts the set verifier with an emergency-stop wrapper (staging/prod), those two entries bypassed the estop — so the upgraded market would keep verifying set-inclusion + assessor proofs even when the legacy market's estop is paused, diverging from the legacy behavior. Resolve the set verifier the same way as groth16/blake3 (router getVerifier, with a fallback to the bare verifier for chains that don't front it) and wrap it in both the set-inclusion and assessor adapters. The set selector is still read from the bare set verifier, since the estop wrapper has no SELECTOR().
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
Adopts the on-chain
BoundlessRoutermodel inside the broker: a request's signed verifier selector resolves to a verifier class, the class'srequiredAssessorClassnames the assessor class, and the broker selects the highest-priority candidate assessor registered there — preferring the nativeOnChainAssessor(an EIP-712 signature) over proving the R0 STARK assessor guest. Previously the guest assessor was wired but effectively unused and the assessor choice was a broker-wide config knob; now it is derived per batch from the router registry, and skipping the guest makes the on-chain assessor actually pay off.flowchart LR A[signed verifier selector] -->|"resolve_verifier_class<br/>(sentinel / class id / entry)"| B[verifier class] B -->|requiredAssessorClass| C[assessor class] C -->|"ASSESSOR_PRIORITY<br/>(first registered wins)"| D{selected assessor} D -->|"0x00000022"| E["OnChainAssessor:<br/>sign FulfillmentBatchAuth,<br/>skip assessor guest"] D -->|"0x00000024"| F["R0 guest:<br/>prove assessor STARK,<br/>set-inclusion seal"]Main changes
SDK (
boundless-market)RouterRegistry: snapshots the router (entries/classes/defaultClassId) once at startup viaload_router_registry; afterwards verifier-class and assessor resolution are pure in-memory lookups — no per-submission RPC.ONCHAIN_ASSESSOR_SELECTOR(0x00000022, stable protocol convention) andR0_ASSESSOR_SELECTOR(0x00000024, version-coupled to the assessor guest the broker ships — same coupling as verifier selectors).build_onchain_assessor_seal/FulfillmentBatchAuth: the EIP-712 seal for theOnChainAssessoradapter.Backend boundary (
boundless-backend)RouterPolicy: backend-agnostic policy over a registry snapshot, parameterized by the backend's capabilities (producible verifier selectors, candidate assessors in priority order). Answers: which verifier selector to emit for a signed selector, which assessor seals a batch, which selectors/classes are supported.Backend::assessor_group(required method): the batch grouping key.Ok(None)= backend doesn't distinguish assessor classes;Err= order unresolvable and must not be batched.RISC0 backend (
risc0-backend)Risc0Backend::from_deployment, which always snapshots the registry (including listen-only mode) — the policy is mandatory, neverOption, so tests and production run the same code path.supported_selectorsonly advertises selectors/classes the backend can produce and seal (a candidate assessor must be registered for the class).Broker
requiredAssessorClassstill co-batch.[B-AGG-602]) instead of silently deferred or batched into an unsealable batch.Tests / wiring
boundless_test_utils::market::test_router_registry: in-memory fixture mirroring the deployed test router topology, so unit tests exercise the real resolution logic over canned data.OnChainAssessor(scripts/localnet-deploy.sh,Manage.Router.s.sol::RegisterOnChainAssessor); broker e2e covers signing the default sentinel, a verifier class id, and specific entry selectors.Stack
Builds on #2005 (OnChainAssessor adapter) and #2020 (legacy ABI fallback), both stacked on #1982 (router decoupling). Until those merge forward, this PR's diff includes the parent branches — the change introduced here is the head commit.