feat(contracts): add OnChainAssessor — native Solidity assessor adapter#2005
Draft
jonastheis wants to merge 11 commits into
Draft
feat(contracts): add OnChainAssessor — native Solidity assessor adapter#2005jonastheis wants to merge 11 commits into
jonastheis wants to merge 11 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.
willpote
approved these changes
May 27, 2026
willpote
left a comment
willpote
left a comment
Contributor
There was a problem hiding this comment.
Overall looks good, few comments
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)
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds the native Solidity
OnChainAssessoradapter — a per-fill predicate-eval + ECDSA-prover-binding implementation ofIBoundlessAssessor— and fixes two correctness gaps surfaced while wiring it end-to-end through the market. Stacked on top of #1982 (jonas/router-decoupling).The adapter
contracts/src/router/adapters/OnChainAssessor.solis stateless and immutable. Per fulfillment batch it vouches for:(claimDigest, fulfillmentData)satisfies the requestor'sPredicate(DigestMatch/PrefixMatch/ClaimDigestMatch).ImageIdAndJournalpayload,(imageId, journal)must reconstruct tofill.claimDigest. Required so a callback dispatched offfulfillmentDatacan't receive bytes the underlying seal never proved.batch.proveris bound via an EIP-712 ECDSA signature over(prover, requestDigests[], claimDigests[]). On-chain equivalent of the R0 STARK adapter'sproverjournal commitment.Brokers select between this adapter and
R0BoundlessAssessorAdapterby setting the first 4 bytes ofassessorSealto the registered adapter's router selector. The market is unchanged — selection happens at the router seam.Correctness fixes uncovered while wiring this in
Predicate.eval(imageId, journal)was usingsha256(abi.encode(journal)), but the off-chain R0 STARK guest,BoundlessMarketCallback, and the broker's claim-digest computation all usesha256(journal)over the raw bytes. The on-chain side never matched a real fill in production. Both call sites inPredicate.solare now aligned with the canonical convention.ClaimDigestMatchcallback binding. The on-chain adapter did not reconstruct the claim digest from(imageId, journal)when the predicate wasClaimDigestMatchand the prover attached anImageIdAndJournalfulfillment payload (typically to feed a callback). The off-chain assessor guest enforces this binding; the on-chain adapter now matches. Without the guard, a malicious prover could submit a valid seal for the requested claim digest while attaching(imageId, journal)bytes that don't match — and the callback would dispatch unproven data.Tests
contracts/test/router/adapters/OnChainAssessor.t.sol— 26 self-contained unit tests covering each predicate kind, the newClaimDigestMatch + ImageIdAndJournalreconstruction guard, predicate failures, length / malformed-seal guards, tamper detection onrequestDigest/claimDigest/fulfillmentData/prover, ERC-165, the domain separator, plus a regression test pinning the type string against the contract'sFULFILLMENT_BATCH_AUTH_TYPEHASHso a silent rename trips loudly.contracts/test/BoundlessMarket.t.sol— wiresOnChainAssessorinto the market's router as a third assessor entry, addscreateFulfillmentBatchOnChain/createFillsAndSubmitRootOnChainhelpers (the EIP-712 analogue ofcreateFillsAndSubmitRootR0), and aBoundlessMarketOnChainAssessorTestcontract exercising locked-request fulfill, batch fulfill, theClaimDigestMatch + ImageIdAndJournalcallback scenarios (both matching and mismatched journal bytes), and a mixed-adapter batch where one fulfillment goes throughOnChainAssessorand another throughR0BoundlessAssessorAdapterin the same transaction.contracts/test/router/BenchBase.sol/AdapterBench.t.sol— bench fixture journal-hash aligned with the canonical convention so adapter benches stay green under the fixedPredicate.eval.