Skip to content

feat: support batch txs#24

Merged
g4titanx merged 24 commits into
masterfrom
feat/support-batch-tx
May 30, 2026
Merged

feat: support batch txs#24
g4titanx merged 24 commits into
masterfrom
feat/support-batch-tx

Conversation

@g4titanx

@g4titanx g4titanx commented May 6, 2026

Copy link
Copy Markdown
Member

Escrow Batch Implementation Summary

What This Implements

EscrowBatch is the batch version of the escrow contract. It lets a deployer define many expected transfers in one escrow, lets one or more nodes reserve different parts of that batch, and lets each node prove that the transfers they reserved were executed before they can collect.

The batch now supports mixed payment assets. A single batch can contain ERC20 transfers and native ETH transfers, for example USDC, USDT, and ETH in the same escrow.

The reward and bond token is still one ERC20 token. That token is set by tokenContract. The payment assets are defined per transfer row.

Transfer Model

Each row in the batch is represented by BatchTransfer:

struct BatchTransfer {
    AssetType assetType;
    address asset;
    address recipient;
    uint256 amount;
    uint256 rewardWeight;
}

assetType says whether the payment is ERC20 or native ETH.

asset is the ERC20 token address for ERC20 transfers. For native ETH, it must be address(0).

recipient and amount describe the transfer the node must execute.

rewardWeight is the common value used to split rewards and calculate bond requirements. This is needed because raw token amounts across USDC, USDT, and ETH cannot be compared directly. For example, 1 ether and 1 USDC are not the same value just because they are both represented as integers on-chain.

So the contract does not use amount to split rewards. It uses rewardWeight.

The contract does not calculate rewardWeight by itself. The deployer or frontend supplies it for each transfer row.

For a batch where every payment uses the same token, rewardWeight can simply match the transfer amount:

100 USDC -> rewardWeight = 100
50 USDC  -> rewardWeight = 50

For mixed-asset batches, rewardWeight should be normalized to a common value, usually USD value or reward-token value:

Transfer A: 100 USDC        -> rewardWeight = 100
Transfer B: 100 USDT        -> rewardWeight = 100
Transfer C: 0.05 ETH at $3k -> rewardWeight = 150

The total weight is:

100 + 100 + 150 = 350

If the total reward is 35 USDC, then the reward split is:

Transfer A reward = 10 USDC
Transfer B reward = 10 USDC
Transfer C reward = 15 USDC

If the deployer wants every transfer row to receive the same reward regardless of payment size, every row can use the same weight:

rewardWeight = 1

Funding

The deployer funds the batch with:

  • the reward token, using tokenContract;
  • all ERC20 payment assets used by the transfer rows;
  • native ETH through msg.value for rows where assetType == NATIVE.

Native funding must exactly equal the sum of all native ETH transfer amounts.

ERC20 payment assets are transferred from the deployer into the escrow contract.

The contract tracks payment balances per asset, so it knows how much USDC, USDT, ETH, or any other supported asset is still locked in the batch.

Funding is one-shot. Once the batch has been funded, it cannot be re-funded with stale state.

Multi-Node Reservations

Nodess do not have to execute the whole batch.

Each node can call bond() with the transfer indexes they want to execute. For example:

  • Node A reserves employees 0 to 9.
  • Node B reserves employees 10 to 19.
  • Node C reserves employees 20 to 29.

The contract prevents overlap. If a transfer index is already reserved or already completed, another node cannot reserve it.

Each reservation stores:

  • the executor address;
  • the reserved transfer indexes;
  • the bond amount;
  • the deadline;
  • the block where the reservation started;
  • the total reward weight reserved by that executor.

The minimum bond is calculated from the executor's share of the reward:

required bond = reward share for reserved rows / 2

The reward share is based on rewardWeight, not raw token amounts.

If a node fails to collect before the deadline, their reservation expires. The reserved transfer indexes become available again, and the forfeited bond is added to the reward pool.

Proof Model

Collection uses one interface:

function collect(BatchProof[] calldata proofs) external;

The array is important because one node may need to prove transfers that landed in different receipts, different transactions, or different blocks.

Each proof has a proofType:

enum AssetType {
    ERC20,
    NATIVE
}

For ERC20 transfers, the proof validates receipt logs.

For native ETH transfers, the proof validates both the transaction and the receipt.

ERC20 Proofs

An ERC20 proof can prove multiple transfer rows from one receipt.

For each ERC20 row, the node provides:

  • the receipt MPT proof;
  • the transfer indexes being proven;
  • the log indexes inside the receipt.

The contract checks that each log proves the exact expected transfer:

  • the token contract must match the row's asset;
  • the from address must be the collecting Nomad;
  • the to address must match the expected recipient;
  • the amount must match the expected amount.

This means a node cannot claim an old transfer, another sender's transfer, or a transfer from the wrong token contract.

Native ETH Proofs

Native ETH does not emit a standard ERC20 Transfer log, so it needs a different proof shape.

For native ETH rows, the contract validates:

  • the transaction MPT proof against the block's transaction root;
  • the receipt MPT proof against the block's receipt root;
  • the receipt status, so failed transactions cannot be claimed;
  • the transaction sender;
  • the transaction recipient;
  • the transaction value.

@ozwaldorf

Copy link
Copy Markdown
Member

failing ci

@g4titanx g4titanx marked this pull request as ready for review May 8, 2026 03:09
@g4titanx g4titanx marked this pull request as draft May 8, 2026 03:09
@g4titanx g4titanx marked this pull request as ready for review May 11, 2026 18:59
@g4titanx

Copy link
Copy Markdown
Member Author

@claude review

@ozwaldorf

Copy link
Copy Markdown
Member

If the deployer wants every transfer row to receive the same reward regardless of payment size, every row can use the same weight: rewardWeight = 1
I'm not sure how I feel about the deployer being the one who determines this, consider implications for reward possibly not reaching all nodes.

We'll also need an efficient way in general to handle possibly under-funding nodes for gas costs. Depending on how many nodes individually work on signals, determines how many extra expensive collect calls there will be. Maybe weight the reward automatically in the escrow side by transfer amount percentages (the same logic as the weights, but using the single number).

@ozwaldorf

Copy link
Copy Markdown
Member

It would be nice to consider incorporating the salt-based commitments from #23 to prevent leaking data if a transaction is never solved

Comment thread src/EscrowBatch.sol Outdated
@g4titanx

Copy link
Copy Markdown
Member Author

@claude review

@g4titanx

Copy link
Copy Markdown
Member Author

It would be nice to consider incorporating the salt-based commitments from #23 to prevent leaking data if a transaction is never solved

once this is merged, we can rebase to #23 and address it there

@ozwaldorf

Copy link
Copy Markdown
Member

@claude review

@claude

claude Bot commented May 14, 2026

Copy link
Copy Markdown

Claude finished @ozwaldorf's task in 3m 12s —— View job


PR Review: feat/support-batch-tx

  • Read CLAUDE.md and repo context
  • Read src/EscrowBatch.sol
  • Read test/EscrowBatch.t.sol
  • Read existing escrow contracts and ReceiptValidator.sol for comparison
  • Analyze code for bugs, security, gas, and design issues
  • Post comprehensive review

Overall this is a solid foundation — the bid lifecycle, swap-and-pop bidder tracking, proof deduplication, and multi-node reservation model are all well-structured. A few significant issues need to be addressed before this is production-safe.


Critical

1. Missing from validation in ERC20 (and native) proofsReceiptValidator.sol:153

// Check second topic (from address) --skip validation
offset = receiptRlp.skipItem(offset);

validateTransferInReceipt explicitly skips from address validation. This means any on-chain ERC20 Transfer to the right recipient with the right amount — from any sender — is accepted as proof. A node could claim reward for a transfer executed by someone else (a coincidental or front-run transfer). The PR description says "the from address must be the collecting Nomad" — but that check is absent.

The same gap exists for native ETH: validateNativeTransfer only checks to and value, not the sender.

Fix this →


2. Broken reward calculation for mixed-asset batchesEscrowBatch.sol:23, 440, 714

The contract already has a // TODO: reward calculation across mixed assets is not fully solved yet. comment, which is the root of this issue. _validateBidIndexes accumulates raw amount values across all assets:

transferAmount += expectedTransfers[transferIndex].amount;

_calculateRewardShare then uses this as a fraction of currentTransferAmount — which itself is a sum of all asset amounts including ETH in wei. In any batch mixing USDC (6 decimals) and ETH (18 decimals), the wei amounts dominate completely, making rewards and minimum bonds nonsensical.

For a [100 USDC, 0.05 ETH] batch: the ETH row contributes 50,000,000,000,000 × more to currentTransferAmount than the USDC row, making USDC's reward share effectively zero. As @ozwaldorf notes, this creates an incentive problem where nodes may ignore low-weight rows.

This is the core design gap. The description proposes rewardWeight to solve it, but neither the field nor the enum AssetType from the PR description appear in the actual struct:

// PR description struct:
struct BatchTransfer {
    AssetType assetType;
    address asset;
    address recipient;
    uint256 amount;
    uint256 rewardWeight;
}

// Actual struct:
struct BatchTransfer {
    address asset;
    address recipient;
    uint256 amount;
}

The implementation is inconsistent with the specification. Either the spec should be updated to reflect the current limitations, or rewardWeight needs to be implemented before this is used with mixed-asset batches.


High

3. extractTransactionsRoot doesn't handle Tempo chain headersBlockHeaderParser.sol:128

extractReceiptsRoot and extractBlockNumber both call getInnerHeaderOffset() which skips the Tempo wrapper on chains 42429/42431. extractTransactionsRoot doesn't — it has its own manual offset from 0:

function extractTransactionsRoot(bytes calldata blockHeader) internal pure returns (bytes32) {
    uint256 offset = 0;
    // Skip RLP list prefix
    ...
    // Skip first 4 fields to get to transactionsRoot (index 4)

On Tempo chains, this reads from the Tempo wrapper fields instead of the inner header, returning garbage data. Native ETH proofs will be completely broken on Tempo chains. Fix this →

4. Native proof uses receiptPath for transaction trie lookupEscrowBatch.sol:561-566

if (!MPTVerifier.verifyReceiptProof(
    batchProof.transactionRlp,
    batchProof.txProofNodes,
    batchProof.receiptProof.receiptPath,  // ← receipt path used for TX trie
    transactionsRoot
))

A transaction and its receipt share the same trie index, so this accidentally works when they are at the same path — but BatchProof has no txPath field, making it impossible to prove a case where the path encoding differs. The struct and validation logic should have a dedicated txPath field.


Medium

5. _handleExpiredBids() is unbounded O(n)EscrowBatch.sol:623

Every call to bid() and collect() runs _handleExpiredBids(), which iterates over the entire bidders array. With enough distinct bidders (each reserving one transfer), this loop can exhaust gas, permanently blocking new bids and collections. Consider a pull model (lazy expiry on-access per bidder) or a gas cap.

6. _validateLogIndexesAreUnique is O(n²)EscrowBatch.sol:592

Nested loop over log indexes per proof. For large ERC20 proofs batching many transfers from one receipt, this is quadratic. A sorted-pair or bitmap approach would be O(n log n) or O(n).

7. Chain-specific dispatch in _sendERC20EscrowBatch.sol:782

if (block.chainid == 11155111) {
    success = IBatchERC20(asset).send(to, amount);
} else {
    success = IBatchERC20(asset).transfer(to, amount);
}

Hardcoding a Sepolia-specific send() interface divergence into the production contract is fragile. If this is needed for a specific token on Sepolia, it belongs in a deployment wrapper or test helper, not in the core contract.


Low / Nits

8. _validateNativeBatchProof return value ignoredEscrowBatch.sol:339

_validateNativeBatchProof(
    batchProof, seenTransfers, seenProofItems, providedTransferCount, activeBid.startBlock
);
// return value (transferIndex) silently dropped

The seenTransfers side-effect works because memory arrays are passed by reference, but the discard is confusing. Consider void return or an explicit _ ignore comment.

9. AlreadyFunded error is misleading after cancelAndWithdrawEscrowBatch.sol:217

After cancelAndWithdraw(), hasBeenFunded is permanently true, so calling fund() again reverts with AlreadyFunded. But the escrow is no longer funded — a more descriptive error (e.g., EscrowExpired or CannotReFund) would be clearer.

10. unchecked usage (reviewer question)

All unchecked { ++i; } and unchecked { ++j; } blocks are exclusively for loop counter increments bounded by array lengths. There is no arithmetic risk. The only accumulations (totalAmount +=, currentAssetPaymentAmount -=) are outside these blocks and remain checked. The pattern is safe.

11. totalTransferAmount is semantically wrong for mixed assets

totalTransferAmount is the sum of all asset amounts in raw units. For a pure single-asset batch this is fine. For mixed assets it's a meaningless number that should not be used for reward math (and currently is).


Test Coverage Gaps

  • No test for collect() with a valid on-chain proof (all collect tests are unit-level validation tests)
  • No test for multi-bidder partial collection followed by second bidder collection
  • No test for the fund() function with native ETH reward asset + native payment transfers
  • No test verifying expired bid bond is correctly added to currentRewardAmount and the new bidder's required bond increases accordingly (only testBidAfterDeadlinePassed partially covers this)

@g4titanx

Copy link
Copy Markdown
Member Author

@ozwaldorf any ideas on reward fee calculations?

Comment thread src/EscrowBatch.sol Outdated
@ozwaldorf

Copy link
Copy Markdown
Member

any ideas on reward fee calculations?

Yes, lets do the initial take where reward percentage is based on tx volumes. Basically use the amount in the current formula, and remove the user-side argument for each tx in the batch.

Comment thread src/EscrowBatch.sol
@g4titanx

Copy link
Copy Markdown
Member Author

any ideas on reward fee calculations?

Yes, lets do the initial take where reward percentage is based on tx volumes. Basically use the amount in the current formula, and remove the user-side argument for each tx in the batch.

already implemented that. this only works cleanly when the amounts are comparable. i mean for same token batch tx it's fine, but for mixed assets, raw amounts aren't comparable

@ozwaldorf

Copy link
Copy Markdown
Member

I wonder if native token should be a separate contract, similar to the single tx contracts, and avoid mixing assets entirely, along with avoiding requiring an oracle or something. Stables are interchangeable (USDC USDT etc)

@g4titanx

Copy link
Copy Markdown
Member Author

one way i can think of right now is to pass normalized per-row value. frontend calculates:
10 USDC -> rewardBasis = 10
1 ETH at say $3000 -> rewardBasis = 3000
then contract uses rewardBasis to split reward

what do you think @ozwaldorf

@g4titanx

Copy link
Copy Markdown
Member Author

I wonder if native token should be a separate contract, similar to the single tx contracts, and avoid mixing assets entirely, along with avoiding requiring an oracle or something. Stables are interchangeable (USDC USDT etc)

yeah true, but we aren't "avoiding mixed assets"

@ozwaldorf

Copy link
Copy Markdown
Member

We should have e2e tests using proofs generated from nomad. You can write a nomad e2e test and have the test dump the proofs it would submit, and plug them into the contract side

@g4titanx

Copy link
Copy Markdown
Member Author

We should have e2e tests using proofs generated from nomad. You can write a nomad e2e test and have the test dump the proofs it would submit, and plug them into the contract side

okay

@g4titanx

Copy link
Copy Markdown
Member Author

one way i can think of right now is to pass normalized per-row value. frontend calculates: 10 USDC -> rewardBasis = 10 1 ETH at say $3000 -> rewardBasis = 3000 then contract uses rewardBasis to split reward

what do you think @ozwaldorf

@ozwaldorf did you see this

@ozwaldorf

ozwaldorf commented May 27, 2026

Copy link
Copy Markdown
Member

I think reward weight is very confusing to use at the moment and changes drastically depending on ETH amounts.

Lets make it rewardRate and computed in the enclave per 1 unit, basically what you had put in the comment previously

USD = rate of 1
ETH = rate of 3000

The node should check the rate and ensure it's acceptable within slippage based on current pricing

@g4titanx

Copy link
Copy Markdown
Member Author

I think reward weight is very confusing to use at the moment and changes drastically depending on ETH amounts.

Lets make it rewardRate and computed in the enclave per 1 unit, basically what you had put in the comment previously

USD = rate of 1 ETH = rate of 3000

The node should check the rate and ensure it's acceptable within slippage based on current pricing

we are no longer using rewardweight though

@g4titanx g4titanx requested a review from ozwaldorf May 29, 2026 14:00
@g4titanx

Copy link
Copy Markdown
Member Author

@ozwaldorf

@ozwaldorf ozwaldorf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should fine tune the reward mechanism in the next pr

@g4titanx

Copy link
Copy Markdown
Member Author

@ozwaldorf i cant merge???!

@g4titanx g4titanx merged commit d8c821c into master May 30, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants