Conversation
- InputSettlerEscrow.refundOnNonFill: revert OutputIndexOutOfBounds(index, length) instead of a generic array-panic when outputIndex is out of range - MandateOutputEncodingLib / OutputSettlerBase: replace magic literals with named FILL_COMMON_PAYLOAD_OFFSET / NOT_FILLED_COMMON_PAYLOAD_OFFSET constants (gas-neutral) - IInputSettlerEscrow: add NatSpec for refund and refundOnNonFill - tests: off-by-one and empty-outputs bounds cases; reverse-direction not-filled/fill domain-crossing rejection Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds receiveSolanaMessage backed by ICrossL2ProverV2.validateSolLogs so fills and non-fills executed on Solana can be attested on EVM via Polymer. Trust model: the remote-oracle identity is taken from Polymer's authenticated returnedProgramId, never from log content, so attestations self-namespace under the emitting program id (honest orders set output.oracle to the trusted Solana program id). Only the trailing base64 field of the "program: <id>, <blob>" log is decoded; blob layout is application(32) || payload. Requires the paired catalyst-intent-svm change (submit emits source||payload, oracle identified by program id) to be deployed together. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- PolymerOracle: correct stale SVM-envelope comment to base64(source||payload); make Solana constants `internal constant`, prover `internal immutable`; use revert-style errors and ++i in the log loop - MockCrossL2ProverV2: centralize proof offsets into named constants and replace require-strings with custom errors - tests: golden-fixture cases for receiveSolanaMessage (base + mapped) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…gramId
Addresses review findings on the Solana proof path:
- Finding 1 (critical): the oracle discarded each log's `program: <id>` prefix and
keyed attestations solely on `returnedProgramId`, never binding the two. Per
Polymer's Solana proof-validation guidance the returned program id MUST equal the
id embedded in every log. Add an on-chain Base58 encoder and require each log to
begin with exactly `"program: " + base58(returnedProgramId) + ", "`, reverting
SolanaProgramIdMismatch otherwise. Prefix is computed once per proof (invariant),
not per log, so multi-log proofs stay within gas limits.
- Finding 3 (low): reject empty payloads (blob length <= 32) instead of attesting
over keccak256("").
- Finding 2 (tests): mock renders the log prefix in base58 (matching the enforced
check); add negative tests asserting a program-id mismatch reverts in both
directions, an empty-payload revert, and Base58 encoder unit tests (golden vector
+ leading-zero rule). A captured real Polymer proof fixture remains a follow-up.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
_openForWithAuthorization previously treated any non-reverting receiveWithAuthorization call as a successful collection; on the single-input branch the raw call's success was trusted without evidence that tokens moved, so a fallback could open a Deposited order with zero funding. Every ERC-3009 collection now requires the escrow's token balance to increase by exactly the input amount, reverting with InvalidBalanceDelta otherwise. A successful single-input call with a wrong delta reverts rather than falling through to the signature-array branch, since the call may already have changed state. Note: escrowOpenFor3009* gas snapshots need regeneration in the canonical toolchain (~2-4k gas increase from two balanceOf calls per input); left untouched here due to local baseline drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Let users open intents paying native ETH directly (token id 0, matching the output-side sentinel) without WETH wrapping, on the escrow settlers only (InputSettlerEscrow + InputSettlerEscrowLIFI). Scope: - open and the SELF branch of openFor become payable and collect native inputs from msg.value via a two-pass _open: pass 1 validates identifiers and requires msg.value == sum of native inputs exactly (checked math; rejects stray ETH on ERC20-only orders and never consults the pooled, force-feedable address(this).balance); pass 2 pulls the ERC20 legs. - All payout paths (finalise, finaliseWithSignature, refund, refundOnNonFill, LIFI fee leg) send ETH for token 0 through a non-virtual _sendInputAsset, so a _transfer override can never mark an order Claimed while paying nothing; zero-amount native sends are skipped. - Permit2 / ERC-3009 branches and the purchase hook reject native explicitly (NativeTokenNotSupported): ETH cannot be pulled by signature, and any path that marked an order Deposited without receiving ETH could drain pooled ETH. - Native-input orders are not purchasable; Tron variants reject native inputs (_nativeInputSupported() overridden to false). - order.user != address(0) enforced at open for every order (a native refund to the zero address would burn ETH; ERC20 refunds already reverted). Push-only payout model: a reverting recipient reverts the whole resolution (no WETH fallback / pull-payment). A hostile order.user contract can block its own refund; a fee-owner contract that rejects ETH blocks native finalisation while a fee is set (unbrick via transferOwnership). Both are accepted, documented, and covered by characterization tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Purchasing a native-input order now works by paying native. The low-level _transferInput pushes ETH (Address.sendValue) for token id 0 instead of reverting, and purchaseOrder is payable. Load-bearing guard: the contract pools all escrowed native, and _transferInput's push draws from that balance, so purchaseOrder requires msg.value to exactly equal the sum of the discounted amounts of the token-0 inputs (and rejects stray value on ERC20-only purchases). Without it a purchaser could pay nothing while the contract paid out ETH escrowed for other orders. Regression-tested by test_purchase_underpayment_cannot_drain_pooled_native_escrow. ERC20 legs still pull via transferFrom; Tron variants still reject native (_nativeInputSupported() == false). Removes the now-obsolete test_native_input_order_is_not_purchasable (supersedes FINDINGS #5 decision that native orders are not purchasable). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…veInputSupported) Drops the _nativeInputSupported() gate and its Tron overrides so native ETH inputs are accepted unconditionally on every escrow variant, including the Tron variants. The signature-path native rejections (Permit2 / ERC-3009) are kept — those reject native because ETH cannot be pulled by signature, unrelated to the removed gate. Inverts the former Tron-rejects-native test into a success test. Note: native support on the real TRON VM (native TRX via Address.sendValue, outside the TRC20 payout hooks) is unvalidated by these EVM-semantics tests and must be validated on a TRON toolchain/testnet before any TRON deployment. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Move _validateUser and the UserIsZero error from InputSettlerEscrow to the common InputSettlerBase so the non-zero-user invariant is reusable; escrow call sites resolve via inheritance (test selector refs updated to base). - Collapse _open's two passes into one loop: per input, validate the clean address and either accumulate the native (token 0) sum or pull the ERC20; the exact msg.value == nativeAmount check runs once after the loop. A wrong value still reverts the whole call (rolling back any pulls); reentrancy stays guarded by the caller's Deposited status flag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… encoders Fold `keccak256(encode*(...))` into single-pass inline-assembly hashers and delete the eight public byte encoders. Production is now hash-only. src: - getMandateOutputHash(+Memory) reimplemented as no-alloc scratch assembly; new hashFillDescription(+Memory) / hashNotFilledDescription(+Memory). - Removed encodeMandateOutput/encodeFillDescription/encodeNotFilledDescription (+ *Memory overloads). Rewired the 7 keccak(encode*) call sites to hash*. - Preimages are byte-identical to the retired encoders. Calldata variants use calldatacopy; memory variants use mcopy (requires a Cancun VM; on Tron the getAllowTvmCancun parameter / java-tron >= Kant v4.8.0). ~27-31% gas per op. test: - New internal wire-format byte builder test/util/RefEncodingLib.sol (no src encoding logic); ~40 raw-byte / hash-preimage sites repointed to it or hash*. - Differential rewritten to prod.hash* == keccak256(independent RefProofDescription) and RefEncodingLib cross-checked; golden vectors pin keccak256(hex) == prod.hash* from both sides; symbolic byte-source repointed. - New MandateOutputHash.equiv.t.sol: boundary matrix (0/word-edges/65535), 10k fuzz, calldata==memory, revert-ordering, common-payload identity, memory-safety canary, gas micro-bench. - Fixed a via-IR block.timestamp caching hazard in test_finalise_self_with_fee (LIFI/tron/compact): read the warp target once, reuse it. Verified: 688 tests pass; differential/golden/symbolic green; coverage compiles on the legacy pipeline (99.93% lines / 96.21% branches / 100% functions); Tron profile builds. Gas snapshots intentionally excluded (separate commit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PolymerOracle assumed validateSolLogs returns "program: <base58>, <base64>" and stripped that prefix, reverting SolanaProgramIdMismatch otherwise. Verified against Polymer's live prover that it strips the entire "Prove: program: <id>, " template and returns only base64(application(32) || payload); the authenticated program id is returned separately as `programID`. The old parse reverted on every real proof — hidden by a mock that fabricated the wrong format. - Treat the whole returned log as the base64 blob; key attestations on the Polymer-authenticated returnedProgramId (program->log binding is provided by the prover's IAVL membership proof). Remove _extractSolanaLogBlob and SolanaProgramIdMismatch. - Fix MockCrossL2ProverV2 to emit the real prefix-free format. - Move the test-only Base58 helper and MockCrossL2ProverV2 out of src into test/util and test/mocks; src/ now ships only the two oracle contracts + vendored prover. - Update tests to the real wire format; drop the now-unrepresentable program-id-mismatch tests; add malformed-base64 and mapped-distinct-chain-id tests; keep self-namespacing / empty-payload coverage. - Add PolymerOracleForkSolana.t.sol + fetch_solana_proof.sh: an env-gated live fork test, validated end-to-end against a real devnet oracle_polymer::submit proof (attests OutputProven under the expected programID/application/payloadHash). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 593a4c6f25
ℹ️ 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".
The LIFI settlers deduct the governance fee in _resolveLock and deliver only the net remainder to the destination, but still handed orderFinalised the gross order.inputs array. A callback that treats those numbers as spendable balances would over-credit or revert once fees are enabled. Add GovernanceFee._netInputs, which reuses _calcFee and the same owner-based fee gate as the claim path, and route all five LIFI callback sites (escrow openForAndFinalise/finalise/finaliseWithSignature and compact finalise/finaliseWithSignature) through it. Base OIF settlers are unaffected (no fee, so gross == net). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…coverage `forge coverage` force-disables viaIR and the optimizer for accurate line attribution, so test_finalise_callback_receives_net_inputs (added in 1308c4b) kept too many locals live through the finalise call and hit stack-too-deep on the legacy codegen path. forge build and forge test were unaffected because [profile.default] sets via_ir = true. Scope the order construction, signature derivation and expected-net computation into blocks so they release before finalise. No behavioural change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
A consolidation pass over the input/output settlers, encoding library, and Polymer
oracle. Removes redundant Tron-specific contracts, collapses the encode-then-hash
layer into single-pass hashers, adds native-ETH support across all escrow paths,
and lands the Solana→EVM proof path for Polymer. Net −1,077/+4,612 (most of the
additions are tests and mocks).
Changes
Input settlers — native ETH + simplification
openFor,and the order-purchase path);
msg.valueis the sole funding source and is checkedfor exact equality.
on that path).
_validateUsertoInputSettlerBase; single-pass_open.InputSettlerEscrowTron.solandSafeTransferLib.tron.sol;Tron behaviour now lives in
InputSettlerEscrowLIFI.tron.sol(USDT payout viaSafeTRC20.safeTransferUSDT, Tron Permit2 address override).Encoding — direct-hash
MandateOutputEncodingLibkeccak256(encode*(...))into no-alloc inline-assembly hashers; delete the eightpublic byte encoders. Preimages are byte-identical to the retired encoders.
hashFillDescription(+Memory)/hashNotFilledDescription(+Memory); ~27–31% gasreduction per op. Memory variants use
mcopy(requires a Cancun VM / java-tron ≥ 4.8.0).Output settler
Polymer oracle — Solana → EVM proofs
receiveSolanaMessage), keyed on the Polymer-authenticatedreturnedProgramId(self-namespacing preserved).application(32) || payload), matching Polymer'slive prover — the previous prefix-parse reverted on every real proof and was masked by a
mock emitting the wrong format.
Base58helper andMockCrossL2ProverV2out ofsrc/intotest/;src/now ships only the oracle contracts + vendored prover interface.oracle_polymer::submitproof.Test & snapshots
MandateOutputHashequivalencetests vs a reference encoder, output not-filled suite, Polymer Solana suites.
snapshots/*.json).Notes for reviewers
refactor is preimage-preserving (equivalence-tested).
mcopy-based hashers require a Cancun-capable VM on every target chain (incl. Tron).🤖 Generated with Claude Code