feat(contracts): forward legacy ABI to old impl via fallback#2020
Draft
jonastheis wants to merge 18 commits into
Draft
feat(contracts): forward legacy ABI to old impl via fallback#2020jonastheis wants to merge 18 commits into
jonastheis wants to merge 18 commits into
Conversation
Adds a frozen copy of the audited OLD market sources under contracts/src/legacy/ to serve as the delegatecall target of the new market's forthcoming legacy-ABI fallback shim. Old brokers continue to hit the deployed OLD impl bytecode via the proxy without translation layers in the new market. File basenames are suffixed with "Legacy" to keep forge artifacts in distinct out/ directories; contract and interface names are unchanged so the compiled bytecode can be byte-matched against the deployed OLD impl in a follow-up. Imports rewritten to point at the renamed files. No other source modifications.
Mirrors the production legacy tree by adding a frozen copy of main's
BoundlessMarket test suite plus the helpers it depends on (TestUtils,
MockCallback, clients/{BaseClient,Client,SmartContractClient,
MockSmartContractWallet}). All imports rewritten to point at
contracts/src/legacy/ for the diverged sources; HitPoints and
BoundlessMarketCallback are reused from src/ since they haven't
changed.
Top-level test contracts are prefixed with "Legacy" (e.g.
BoundlessMarketBasicTest -> BoundlessMarketLegacyBasicTest) so gas
snapshots land in their own JSON files instead of overwriting the new
market's. 133 tests pass.
… impl
Pins contracts/src/legacy/ to the audited BoundlessMarket implementation
currently deployed on Base mainnet (0x22bb...cd3 behind proxy
0xfd15...fe82). The check ensures the frozen tree continues to compile to
the same bytecode as the audited deployment, so the new market's
forthcoming fallback() can delegate-call into it without revisiting the
audit for the legacy ABI surface.
Adds:
- contracts/test/legacy/deployed-bytecode.hex: snapshot of the deployed
runtime bytecode (24,371 B), pulled at Base block 46,576,272.
- contracts/test/legacy/deployed-bytecode.meta.toml: provenance plus
expected constructor immutable values (VERIFIER, ASSESSOR_ID,
COLLATERAL_TOKEN_CONTRACT, DEPRECATED_ASSESSOR_*, APPLICATION_VERIFIER).
- contracts/scripts/verify-legacy-bytecode.py: stdlib-only verifier that
(1) masks all immutable slots and asserts the rest matches byte-for-byte
and (2) extracts each declared immutable's baked value from the
deployed bytecode and asserts it matches the expected meta values.
Skips the inherited UUPS __self immutable (always address(this)).
- .github/workflows/contracts.yml: new legacy-bytecode-parity job,
gated by existing src/foundry/test/scripts/ci path filters.
- justfile: check-legacy-bytecode recipe, wired into the umbrella check.
- license-check.py: legacy/IBoundlessMarketLegacy.sol on APACHE_PATHS to
mirror its src/ counterpart's license header.
…/legacy/
Adds an invariant check that the new market and the frozen legacy market
agree on every storage slot reachable from both ABIs. Delegate-calling the
legacy impl from the new market's forthcoming fallback only works if each
(slot, offset, width) the legacy code touches means the same thing in the
new market's view of storage; this guards against silent drift if either
tree adds, removes, or reorders state fields.
The verifier compares forge-emitted storageLayout for both artifacts:
1. Top-level variables at slot 0/1/2 (requestLocks, accounts, imageUrl)
agree on label, slot, offset, and normalized type.
2. Every struct transitively referenced from those slots (Account,
RequestLock) has identical member layouts in both contracts.
AST id suffixes embedded in type identifiers are stripped before
comparison so two artifacts with different compile-time ids still match
when their underlying type names do. Wired into `just check` and the
existing contracts-CI job.
Adds a LEGACY_IMPL immutable to BoundlessMarket plus a payable fallback()
that delegate-calls into it for any selector the new contract does not
declare. The legacy ABI (Fulfillment[] + AssessorReceipt shape, the four
submitRootAnd* variants, etc.) is preserved without re-introducing the
bodies into the new market: in-flight transactions and old broker
clients stay functional during the migration window, while the new code
path goes through the BoundlessRouter as before.
The constructor now takes the legacy impl address as a third argument.
On Base mainnet this is the pre-upgrade implementation pointed to by
the proxy; the deployed bytecode there is already audited (see
contracts/scripts/verify-legacy-bytecode.py for the parity invariant).
On dev/localnet, tests and deploy scripts stand one up from
contracts/src/legacy/BoundlessMarketLegacy.sol.
Plumbing:
- BoundlessMarket.sol: new error InvalidLegacyImpl, new constructor
arg + zero-address check, public immutable LEGACY_IMPL, payable
fallback that calldatacopy / delegatecall / returndatacopy /
revert-or-return.
- BoundlessMarketLib.encodeConstructorArgs: extended to include
legacyImpl for the OZ upgrades safety checks.
- Deploy.s.sol / Manage.s.sol: read BOUNDLESS_LEGACY_IMPL env var,
thread through constructor + encode, assert the deployed market's
LEGACY_IMPL() matches, payable() casts on BoundlessMarket(addr)
conversions (required now that the type has a payable fallback).
- BoundlessMarket.t.sol: setUp deploys a BoundlessMarketLegacy impl
first and feeds the address into every BoundlessMarket constructor.
Runtime size: 29,456 -> 30,293 B (+837). Larger than a bare assembly
shim because the public LEGACY_IMPL getter and the dispatcher tail
change pull in more than just the fallback body. Still over EIP-170;
tracked separately on the size-shrink branch.
353 non-legacy tests pass; bytecode-parity and storage-layout invariants
remain green.
Clones the legacy test suite into BoundlessMarketLegacyViaFallback.t.sol and rewires setUp so the proxy points at the new market impl while LEGACY_IMPL points at a fresh legacy impl deployed alongside. Tests are typed against the legacy BoundlessMarket so every call emits the legacy ABI selectors; selectors the new market declares (lockRequest, slash, withdraw, view getters, etc.) execute on the new impl, while legacy-only selectors (fulfill with the old shape, imageInfo, verifyDelivery, the submitRootAnd* variants with AssessorReceipt) fall through to the legacy impl via fallback(). This is the load-bearing validation of the architecture: 133 tests pass end-to-end, proving that (a) the fallback routes legacy selectors correctly, (b) storage interop holds between the two impls in both directions, and (c) the new market is behaviorally compatible with the legacy on every shared method the legacy suite exercises. Two regression tests had their hard-coded recovered-address assertions re-baselined: the proxy address shifts by one CREATE nonce because setUp now deploys an extra contract before the proxy, which shifts the EIP-712 domain separator and therefore the deterministic garbage address that ECDSA.recover produces for a malformed signature. Annotated inline. Test contracts are prefixed with "ViaFallback" so their gas snapshots land in their own JSON files rather than overwriting the standalone legacy suite's.
Adds CrossABI.t.sol next to the via-fallback suite. Inherits its setup
(new market behind the proxy, legacy impl as fallback target) and
isolates the four behaviors that are unique to this deployment shape:
- testLegacyImageInfoViaFallback: legacy-only view callable through
fallback; returns the legacy impl's baked-in ASSESSOR_IMAGE_ID and
an empty imageUrl since the new market's initialize(address)
signature does not set it.
- testLegacyImmutablesReadableViaFallback: VERIFIER() and ASSESSOR_ID()
getters auto-generated from legacy immutables route through fallback
and return values baked into the legacy bytecode.
- testFallbackRevertsOnUnknownSelector: a selector missing on both
contracts reverts cleanly through the fallback's delegatecall path.
- testSharedViewReadsLegacyFulfilledState: lock via the shared
lockRequest (executes on the new impl), fulfill via the legacy
fulfill(Fulfillment[], AssessorReceipt) selector (executes on the
legacy impl via fallback), then read state via shared
requestIsFulfilled (executes on the new impl). Confirms writes from
the legacy impl are visible to the new impl's view — the
load-bearing storage interop invariant.
The contract docstring captures why the rest of the originally-planned
matrix is moot: shared selectors always run on the new impl regardless
of which "ABI" the caller meant to use, so a "lock legacy vs lock new"
distinction does not exist on-chain and is already covered by the
via-fallback suite.
…oyment-test Deploy.s.sol now resolves the legacy impl address optionally: if BOUNDLESS_LEGACY_IMPL is set in the environment (production / mainnet upgrade paths) use it as-is, otherwise deploy a fresh legacy impl from contracts/src/legacy/ wired to the same verifier, applicationVerifier, assessor image id, and collateral token the new market is about to use. The address is threaded into the new market's third constructor arg. Manage.s.sol's production path still requires BOUNDLESS_LEGACY_IMPL to be set explicitly, so operators cannot accidentally ship a freshly-deployed unaudited legacy impl to mainnet. The deployment-test profile (contracts/deployment-test/Deploymnet.t.sol) was already broken on this branch because the router-decoupling work removed AssessorReceipt and related types from src/types/. Redirected its imports to contracts/src/legacy/. After the upgrade those legacy-shape calls reach the deployed market through the new fallback, so the test shape matches the production path. FOUNDRY_PROFILE= deployment-test forge build is clean again. New-ABI coverage of the deployed market is a follow-up.
Adds LEGACY-FROZEN.md alongside the frozen tree, capturing what the folder is, what's in it, where it came from, and what to do when something needs to change. Records the source provenance (main commit 507f746, the last commit on main that touched any of the files mirrored here), the on-chain identity the bytecode must continue to match (Base mainnet impl 0x22bb6bbe5d221ef3e738029dab4d1d27ec725cd3), and the diff command reviewers can run to verify each file's provenance. Names the two CI invariants that keep the tree honest (verify-legacy-bytecode.py and verify-storage-layout.py, both in the legacy-bytecode-parity job) and states the freeze policy: no modifications. If the deployed impl genuinely changes (CVE, compiler bump, sunset), refresh deployed-bytecode.hex + .meta.toml and re-run just check-legacy-bytecode.
willpote
approved these changes
Jun 1, 2026
Replace the hand-rolled assembly fallback() with inheritance from OpenZeppelin's Proxy: override _implementation() to return LEGACY_IMPL and let the inherited fallback() delegate-call into it. Same delegatecall semantics, now backed by audited OZ code. Costs +263 B of runtime bytecode (the OZ fallback -> _fallback -> _delegate indirection does not fully inline under via-ir at 100 runs), accepted as a reuse/readability tradeoff. Addresses review feedback on #2020.
The via-fallback suite was a ~4.4k-line near-verbatim copy of the standalone legacy suite, differing only in how the market under test is deployed and in two signature-recovery expectations. Extract the deployment into a virtual _deployMarket() hook and the two recovered-signer addresses into virtual getters on the legacy base. The via-fallback file becomes a thin BoundlessMarketLegacyViaFallbackTest base that only overrides _deployMarket(), plus subclasses that inherit the full test battery. Foundry runs both the legacy and via-fallback contracts, so each entry point keeps full coverage. Net -4,384 lines. Addresses review feedback on #2020.
…ctor The legacy-fallback work added a required `legacyImpl` constructor arg (reverts on zero), but the Rust bindings were never regenerated: build.rs still declared the 2-arg constructor, so the test harness deployed with a zero legacyImpl and every create_test_ctx-based test reverted at deploy. Update the generated constructor signature and pass a non-zero stub. The legacy fallback is only for pre-router clients hitting the deployed contract; current code must never route through it, so tests use a non-functional placeholder rather than a real BoundlessMarketLegacy.
dprint: reflow LEGACY-FROZEN.md (escape the leading `+` so it isn't parsed as a list bullet, left-align the architecture diagram, align the table). forge fmt: wrap the multi-line import in CrossABI.t.sol and minor spacing in Deploy.s.sol / Manage.s.sol.
DeployBoundlessMarket and UpgradeBoundlessMarket required BOUNDLESS_LEGACY_IMPL via vm.envAddress, reverting on dev / localnet / fresh networks where it isn't set, which broke the deployment-scripts CI job. DeployBoundlessMarket now deploys a fresh BoundlessMarketLegacy from the configured verifier/token when the env var is unset; UpgradeBoundlessMarket defaults to the deployed market's existing LEGACY_IMPL. The deployment-scripts CI job sets BOUNDLESS_LEGACY_IMPL to a non-functional stub so the e2e exercises the new ABI without a live legacy fallback.
The deployment-test imported the legacy BoundlessMarket + legacy types but fulfilled via the new batched priceAndFulfill, so it neither compiled nor exercised the legacy fallback. Point all imports at src/ (new contract + new types) so it consistently tests the new market via the batched ABI. Legacy fallback coverage is left to a separate e2e test.
The harness deployed a real BoundlessMarketLegacy solely to satisfy the new market's non-zero legacyImpl guard; it never calls the legacy ABI (that lives in test/legacy/). Point legacyImpl at a non-functional stub instead, so the new-ABI suite never depends on the legacy contract and an accidental fallback call fails instead of silently working.
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
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
The router-decoupling rework on
jonas/router-decouplingreplaces themarket's pre-router fulfillment ABI with a slimmer
FulfillmentBatch[]shape and routes verification through
BoundlessRouter. Old brokers willkeep submitting the original
(Fulfillment[], AssessorReceipt)shape forthe duration of the broker rollout, so the deployed market must continue
to accept legacy calls without us rewriting or re-auditing the old
fulfillment logic.
This branch wires up a
fallback() external payableon the new marketthat delegate-calls into the already-deployed audited implementation
at the proxy's pre-upgrade impl address. Nothing new is written for the
legacy ABI surface — the audited bytecode handles it byte-identically to
pre-upgrade, just delegate-called. The only new on-chain code is the
fallback forwarding — the new market inherits OpenZeppelin's
Proxyandoverrides
_implementation()to returnLEGACY_IMPL— plus theLEGACY_IMPLconstructor immutable.A frozen copy of the audited source lives at
contracts/src/legacy/, usedexclusively for tests and CI parity invariants — production deployments
point
LEGACY_IMPLat the existing impl on Base mainnet. The legacysources mirror
mainat commit507f7469(
BM-2598: add depositCollateralTo and depositCollateralWithPermitTo,the last commit on
mainthat touched any of the files in this tree).The only deltas are file-basename + import renames; contract bodies are
unchanged.
The new market's own test harnesses (the Solidity unit suite, the Rust
test harness, and the
deployment-scriptse2e) pointLEGACY_IMPLat anon-functional stub address, so a current-ABI call can never be silently
served by the fallback — any accidental fallback hit fails instead of
working. The real legacy impl is stood up only by the dedicated
test/legacy/suites (which exercise the fallback on purpose) and by thedeploy scripts on fresh networks.
Architecture
Methods that actually exercise the legacy contract
Forwarded through
fallback()to the audited legacy impl. Everything inthe list below has a different selector on the new market (different param
types) or doesn't exist on it at all:
fulfill(Fulfillment[],AssessorReceipt),fulfillAndWithdraw(...),priceAndFulfill(ProofRequest[],bytes[],Fulfillment[],AssessorReceipt),priceAndFulfillAndWithdraw(...)submitRootAndFulfill(addr,bytes32,bytes,Fulfillment[],AssessorReceipt),submitRootAndFulfillAndWithdraw(...),submitRootAndPriceAndFulfill(...),submitRootAndPriceAndFulfillAndWithdraw(...)setImageUrl(string), legacyinitialize(address,string)verifyDelivery(Fulfillment[],AssessorReceipt),imageInfo(),VERIFIER(),APPLICATION_VERIFIER(),ASSESSOR_ID(),DEPRECATED_ASSESSOR_ID(),DEPRECATED_ASSESSOR_EXPIRES_AT(),DEFAULT_MAX_GAS_FOR_VERIFY()Everything else —
lockRequest,lockRequestWithSignature,slash,withdraw, all thedeposit*/depositCollateral*variants,submitRoot, every view getter shared with the legacy ABI, everyAccessControl role method — has the same selector on the new market
and therefore runs on the new impl, even when invoked from a legacy
broker. The legacy impl's body for those methods is unreachable from the
proxy; the audited-bytecode-reuse story only covers the methods in the
table above. Shared methods are covered by the new impl's audit.
Gas impact
The fallback path adds one delegate-call hop + calldata/returndata copy on
every legacy-only call.
Per-method overhead (via-fallback
BoundlessMarketLegacyViaFallbackBasicTestgas snapshot vs. the standalone legacy suite running identical scenarios):
fulfill: a locked requestfulfill: a locked request with 10kB journalfulfill: another prover fulfills without paymentfulfillAndWithdraw: a locked requestsubmitRootAndFulfill: a locked requestsubmitRootAndFulfillAndWithdraw: a locked requestMean overhead across the 43 measured methods: +566 gas. Typical
fulfillment path: ~2.2k gas extra. Worst case (10kB journal): ~4.4k gas
extra, dominated by the returndatacopy of the large output.
A handful of batch-fulfill rows show a small negative delta (a few
hundred gas faster via fallback). That's noise from different CREATE
nonce ordering in the test setups (cold/warm state on participating
contracts differs); the actual per-call overhead is positive and bounded
by the delegate-call cost.
Implementation runtime bytecode: 29,456 → 30,556 B (+1,100 B). The
overshoot vs the originally-budgeted ~50 B comes from the public
LEGACY_IMPL()getter the immutable generates, the dispatcher's new tailchanging optimizer inlining decisions, and the OpenZeppelin
Proxyindirection (+263 B vs the prior hand-rolled assembly fallback —
via-irat 100 runs doesn't fully inline
fallback() → _fallback() → _delegate()back down to the bare
delegatecall). Still over EIP-170; the size shrinkto fit under the cap is tracked separately on
jonas/market-bytecode-shrink.How to review
The largest commit by line count is the legacy source / test imports.
Reviewing those line-by-line is wasteful — they're frozen copies of
main's sources. Use the commands below to confirm there are nodifferences vs
mainbeyond the explicit renames (file basenames andimport paths).
1. Production source tree (
contracts/src/legacy/)The renamed root contracts:
Expected diffs: import-path strings rewritten to point at the renamed
files inside
legacy/. Nothing else.The frozen library + type subtrees should be byte-identical to
main:The two commands above should produce no output. If they do, that's
where to focus the source review.
2. Test tree (
contracts/test/legacy/)The standalone-legacy port of the test suite:
Expected diffs: import paths rewritten to
legacy/, the four top-leveltest contracts prefixed with
Legacyso gas snapshots land in their ownJSON files.
Helpers (frozen):
Expected diffs: import paths only (
../src/→../../src/legacy/or../../src/).The via-fallback port (
BoundlessMarketLegacyViaFallback.t.sol) is nolonger a copy of the suite — it subclasses it, so review the whole file
directly (~110 lines). The standalone-legacy base now exposes a
virtual _deployMarket()hook andvirtualgetters for the two signature-recoveryaddresses; the via-fallback file is a thin
BoundlessMarketLegacyViaFallbackTestbase that overrides
_deployMarket()(deploy the new market withLEGACY_IMPLpointed at a freshly-deployed legacy impl) plus subclassesthat inherit the full battery and re-baseline the two recovered addresses
for the shifted proxy address. Foundry runs both the standalone and
via-fallback contracts, so each entry point keeps full coverage.
3. Bytecode-parity invariant
The truly load-bearing check that the legacy tree is the audited code:
The first asserts
legacy/BoundlessMarketLegacy.solcompilesbyte-identical to
cast code 0x22bb...cd3 --rpc-url $BASE_RPC(modulomasked immutable slots; values cross-checked against
contracts/test/legacy/deployed-bytecode.meta.toml). The second assertsthe new and legacy storage layouts agree at every slot reachable from
either contract.
Both run automatically in CI (
legacy-bytecode-parityjob in.github/workflows/contracts.yml).4. Commits to review individually
The substantive changes are in these commits — review the diffs directly:
feat(contracts): forward legacy ABI to a configurable impl via fallbackBoundlessMarket.sol(LEGACY_IMPL,fallback()),BoundlessMarketLib, every constructor call site + payable castschore(contracts): verify legacy/ bytecode parity against deployed OLD implchore(contracts): enforce storage layout interop between src/ and src/legacy/test(contracts): pin cross-ABI invariants in a focused suiteCrossABI.t.sol— the 4 invariants unique to the fallback shapechore(contracts): wire legacy impl into Deploy.s.solBOUNDLESS_LEGACY_IMPLenv varrefactor(contracts): forward legacy ABI via OpenZeppelin Proxyfallback()with inheritance from OZProxy+ an_implementation()override (review feedback)refactor(contracts): de-duplicate legacy ABI test suites via inheritancefix(test-utils): pass legacyImpl to the 3-arg BoundlessMarket constructorbuild.rs/bytecode.rs) for the 3-arg constructor and points the Rust test harness'slegacyImplat a non-functional stubfix(contracts): resolve legacyImpl in manage deploy/upgrade scriptsManage.s.solDeployBoundlessMarket/UpgradeBoundlessMarketresolvelegacyImpl(deploy-if-zero from config / preserve the deployed market's existingLEGACY_IMPL); thedeployment-scriptsCI job setsBOUNDLESS_LEGACY_IMPLto a non-functional stubtest(contracts): migrate deployment-test to the new batched ABIdeployment-test/Deploymnet.t.solexercises the new market via the batched fulfill (it had imported legacy types while calling the batched API)test(contracts): stub the legacy impl in the BoundlessMarket harnessBoundlessMarket.t.solpointslegacyImplat a non-functional stub instead of deploying a realBoundlessMarketLegacy— the suite never calls the legacy ABI (that lives intest/legacy/)