Repository navigation
Conversation
heifner
left a comment
There was a problem hiding this comment.
The decision table and the fail-closed cursor reads are good to see, and dropping the staging machinery is a real simplification. Two things I think need attention before this lands, with details inline:
- Gas funding depends on anvil's estimator. The floor assumes
eth_estimateGasreturns the least gas at which the call succeeds. anvil does; geth and reth bound the search from below by the gas used at the top of the range, so anepochInthat spills estimates near the 16,777,216 cap and the 20% buffer then pushes it over the cap, or over a finitemax_gas_limit. The continuation path can't engage on those clients. - The continuation loop never checks that the cursor moved. An under-funded continuation is a successful no-op on the contract side, so the relay keeps sending.
docs/ethereum-client-config.example.jsonships"max_gas_limit": "2000000", which is below what the emit attempt needs.
Outside the diff:
plugins/outpost_ethereum_client_plugin/README.mdstill documents the chunked protocol: the members table, the "Outbound delivery and chunking" section, the log lines and the test list.outpost_client::deliver_outbound_envelope's doc still says the consensus-reaching transaction performs the emit and that the return is a transaction id, and the comment inoutpost_opp_job::run_outboundabout the empty id still refers to chunked Ethereum deliveries.- The description cites
test_fc -t ethereum_transaction_policy_tests. That suite is the libfc file, which this PR doesn't touch; the changed file is theoutpost_ethereum_transaction_policy_testssuite intest_outpost_ethereum_client_plugin. tests/fixtures/ethereum-abi-opp-inbound-current.jsonlooks like it predates the last commits on Wire-Network/wire-ethereum#216: it still haslastMessageIDandpendingMessageCountand lacksinstallInitialRoster,FIRST_INBOUND_EPOCH_INDEXandOPP_BootstrapWindowClosed. None of the bound entries is affected.
| auto estimated_gas = estimate_gas(to, contract, data, gc); | ||
| auto gas_limit = derive_buffered_gas_limit(_transaction_policy, estimated_gas); | ||
| if (gas_limit_floor != 0) { | ||
| // The floor is bounded by the same ceiling as the estimate: a caller | ||
| // asking for more than the policy allows is refused, not clamped, so | ||
| // an under-funded call is never silently sent. | ||
| const fc::uint256 floor{gas_limit_floor}; | ||
| if (floor > _transaction_policy.max_gas_limit) { | ||
| throw_transaction_policy_exception(ethereum_transaction_policy_reason::gas_limit_cap_exceeded, | ||
| transaction_policy_field::gas_limit_floor, | ||
| floor.str(), | ||
| _transaction_policy.max_gas_limit.str()); | ||
| } | ||
| if (gas_limit < floor) gas_limit = floor; | ||
| } |
There was a problem hiding this comment.
This ends up as max(estimate x 6/5, floor), and derive_buffered_gas_limit throws before the floor is looked at when the buffered estimate is over max_gas_limit.
The comments say eth_estimateGas converges on the least gas at which the call succeeds. That is anvil's behaviour (it bisects up from intrinsic gas). geth and reth run the call at the top of the range and then use the gas that run used as the lower bound: lo = result.UsedGas - 1 in geth's gasestimator.go, under a comment about calls that check gas remaining, and lowest_gas_limit = gas_used.saturating_sub(1) in reth. The top of the range is capped at 16,777,216 under Osaka.
An epochIn that spills runs until less than DispatchGasFloor is left, so its estimate is about 15.8M or more and x 6/5 is about 19M. With a finite max_gas_limit that is gas_limit_cap_exceeded; with the maximum policy it gets signed at about 19M and the node refuses it. Anything using more than about 13.98M at the estimator's ceiling can't be sent, which includes every envelope that needs a continuation. e2e runs on anvil, so it can't show this.
Could the caller supply the gas limit rather than a floor? When set: skip the buffer, use it as the limit (still policy-checked), and pass it as gas in the eth_estimateGas request so the node simulates the transaction that will be sent. That also keeps the free revert check accurate, since today the node simulates at its own ceiling rather than at the limit we send.
There was a problem hiding this comment.
Done as you suggested: with a cap set, the cap is the limit, the buffer and the buffered-ceiling check are skipped, and the estimate runs with gas = cap. 8caeb85.
| case detail::delivery_action::continue_dispatch: | ||
| break; |
There was a problem hiding this comment.
Nothing checks that the previous continuation moved the cursor. On the contract side an under-funded continuation is a successful no-op by design: _tryEmitOutbound returns false when less than EmitAttemptGasFloor (4,000,000) is left, and _dispatchFrom returns (0, false) when a continuation starts with less than DispatchGasFloor. The receipt is status 1, the next read is identical, and we send the same call again.
With a policy max_gas_limit below roughly 4.1M the emit is never attempted, so every epoch ends up here, and docs/ethereum-client-config.example.json has "max_gas_limit": "2000000". On real block times the tick ends in the deadline throw, the job doesn't mark the epoch handled, and the relay sends about one paid no-op per tick.
Suggest keeping (dispatched, complete) from before each continuation and, when the next read is still tipped, unfinalized and unchanged, logging the cause at error level and returning. The example config needs a larger value, and it would help to document that max_gas_limit now has a practical minimum for epochIn.
There was a problem hiding this comment.
Added advanced(before, after): a confirmed call that moved nothing escalates, and at the ceiling logs once per cursor position and ends the tick in outpost_delivery_incomplete_exception. Example config raised to 2^24; README documents the 9 932 160 minimum and the relay refuses a client below it.
| wlog("outpost_ethereum_client[{}]: epoch={} still not finalized after {} continuations in " | ||
| "one tick; the next tick resumes from the outpost's cursor", | ||
| to_string(), epoch_index, MAX_CONTINUATIONS_PER_TICK); | ||
| return last_tx; |
There was a problem hiding this comment.
outpost_opp_job::run_outbound marks the epoch handled on any non-throwing return and then calls again only once, after the depot boundary. So the next tick doesn't resume from here; the comment above the loop has the same assumption, which holds only for the throwing exits. Two smaller things: the loop runs 33 sends (<=), and nothing is read after the last one, so "still not finalized" isn't known.
I'd read once more after the last send and throw if the epoch is still open, as the deadline path does. That is only safe together with the progress check, otherwise it turns a bounded loop into a per-tick one.
There was a problem hiding this comment.
Loop is now < with a read after the last send; still open throws the same exception, which the job logs at info and does not mark handled.
| const auto next_epoch = read_next_epoch_index(); | ||
| const auto spill = read_dispatch_spill(epoch_index); | ||
| // The two settlement reads are only meaningful — and only paid for — | ||
| // when the epoch is mid-flight. | ||
| const bool settled = spill.tipped && !spill.finalized && own_delivery_settled(epoch_index); |
There was a problem hiding this comment.
These are four separate eth_calls at latest, so behind a load-balanced endpoint they can be answered from different blocks, or from a backend that hasn't seen the block whose receipt we just got. The case that costs something: right after this relay's own tipping transaction a stale read sees "not tipped" (or a zero epochDeliveries), we return normally, and the job marks the epoch handled. The other deliverers returned earlier, so a spilled epoch then waits for the post-boundary retry, and the depot can't advance until it finishes.
latest is the right tag here. Could the four views be pinned to one block number per iteration, at or after the last receipt's block? block_number_or_tag_t already takes a number string. Longer term I think the job should keep calling until the outpost reports the epoch closed, since the reads are free, but that can be a follow-up.
Related: the doc on read_dispatch_spill says the worst a reorg can cost is a re-sent no-op. A re-send is a paid re-delivery before the tip and a revert from a relay that hasn't delivered once the epoch has tipped, and the costlier direction is the send that isn't made.
There was a problem hiding this comment.
All four views are pinned to one block number per iteration: head at tick start, then each receipt's block. Reorg-cost doc reworded as you put it. Keep-calling-until-closed I'd take as a follow-up: a job refactor (delivered vs closed) plus a Solana touch.
| std::optional<bool> abi_bool_output(const fc::variant& value) { | ||
| if (value.is_bool()) return value.as_bool(); | ||
| if (value.is_string()) { | ||
| const auto text = value.as_string(); | ||
| if (text == "true") return true; | ||
| if (text == "false") return false; | ||
| } | ||
| const auto numeric = abi_uint_output(value); | ||
| if (numeric) return *numeric != 0; | ||
| return std::nullopt; | ||
| } |
There was a problem hiding this comment.
decode_static_value returns a native bool for dt::boolean, so only the first line is reachable, and the one caller asserts on nullopt anyway. The Solana client's equivalent read just calls as_bool(). Could this be as_bool(), or FC_ASSERT(v.is_bool(), ...) if you want the strict form?
Same idea for the two helpers below. decoded_scalar's array and object branches can't be reached for single-output getters, since contract_decode_data returns the bare value. normalized_word repeats strip_hex_prefix, and the comparison could be same_hex.
There was a problem hiding this comment.
Removed both; is_bool() assert and the bare decoded value. normalized_word kept for topics and log data; comparisons go through same_hex.
| case detail::delivery_action::wait_for_deliverer: | ||
| ilog("outpost_ethereum_client[{}]: epoch={} tipped on a digest this relay did not " | ||
| "deliver (dispatched={} complete={}); its deliverers carry the continuation", | ||
| to_string(), epoch_index, spill.dispatched, spill.complete); | ||
| return last_tx; |
There was a problem hiding this comment.
own_delivery_settled is false both when this relay never delivered and when it delivered a different digest. The second means our envelope disagrees with the majority, and it gets the same ilog. Could it tell the two apart and wlog the mismatch with both digests?
There was a problem hiding this comment.
Settlement is a four-way enum; divergent gets a wlog with both digests.
| /// Decoded `OPPInbound.dispatchSpill(uint32)` view result — where a tipped | ||
| /// epoch's processing stands. All-false/zero for an epoch that has not tipped, | ||
| /// which is also what an unreadable response decodes to. |
There was a problem hiding this comment.
read_dispatch_spill throws on an unreadable response rather than returning all-false, so the last clause no longer holds.
| // SIZING RULE for batch-delivery-timeout-ms: it bounds the WHOLE outbound | ||
| // delivery, and an Ethereum delivery is now one transaction PER CHUNK | ||
| // (ETHEREUM_MAX_CHUNK_BYTES = 8192; Solana chunks at 672). Size it as | ||
| // (whole-envelope on Ethereum, continued while it spills; Solana chunks at 672). Size it as |
There was a problem hiding this comment.
The rest of this comment still describes chunking: "one transaction PER CHUNK", "total chunks x ...", "4 Ethereum chunks", "per-operator high-water mark", "raise it for chunking". Something like: Ethereum sends the envelope in one transaction plus one more per continuation while dispatch spills; Solana sends one per 668-byte chunk; size it as transactions x (block time + margin).
One thing that affects the sizing: each receipt wait is also capped by retry_option_defaults.total_timeout (15 s), which delivery_confirm_options copies, so raising this option doesn't lengthen a single wait.
There was a problem hiding this comment.
Rewritten for one delivery plus continuations, resume from dispatchSpill.
| inline uint64_t delivery_gas_limit_floor(const ethereum_transaction_policy& policy) { | ||
| const fc::uint256 cap{EIP_7825_TX_GAS_CAP}; | ||
| return policy.max_gas_limit < cap ? policy.max_gas_limit.convert_to<uint64_t>() : EIP_7825_TX_GAS_CAP; | ||
| } |
There was a problem hiding this comment.
Worth documenting for operators: max_gas_limit is now the gas limit of every epochIn, including a cheap non-tipping first delivery. The signer needs gas_limit x max_fee_per_gas free for each one (about 0.7 ETH at 16,777,216 gas and a 20 gwei base fee), max_total_native_cost_wei has to be at least max_gas_limit x max_fee_per_gas_wei, and a default geth refuses the submission when gas limit x maxFeePerGas is over its 1 ETH --rpc.txfeecap (about 59.6 gwei at this limit).
There was a problem hiding this comment.
README now has the per-call reservation, the ~0.7 ETH figure, the max_total_native_cost relation and geth's --rpc.txfeecap.
| // OPPInbound's one write wrapper — the whole-envelope delivery — must be | ||
| // rejected by the policy before signing. It is funded to the policy ceiling | ||
| // (999 here), which the ×1.2-buffered estimate of 834 already breaches. | ||
| uint32_t epoch_index = 1; | ||
| uint16_t chunk_index = 0; | ||
| uint16_t total_chunks = 1; | ||
| uint32_t total_bytes = 1; | ||
| std::string chunk = "01"; | ||
| expect_policy_rejection( | ||
| [&] { inbound.epoch_in(epoch_index, chunk_index, total_chunks, total_bytes, chunk); }); | ||
| expect_policy_rejection([&] { inbound.discard_envelope_chunks(); }); | ||
| std::string envelope = "01"; | ||
| expect_policy_rejection([&] { inbound.epoch_in(epoch_index, envelope); }); |
There was a problem hiding this comment.
This is the only test that calls the real epoch_in wrapper, and it is rejected on the buffered estimate before the floor is consulted. So as far as I can see nothing pins that epochIn is funded to the floor: dropping delivery_confirm_options(client) from the wrapper would still pass. A case where the floor decides the outcome would cover it.
Other gaps: loop exhaustion, a throwing epoch_in on the first send and on a continuation, the fail-closed paths in the three cursor readers, and an outpost_opp_job case where the client returns with the epoch tipped but unfinished.
There was a problem hiding this comment.
The wrapper case now sends at exactly the budget and pins the estimate's gas; new cases cover loop exhaustion, a throwing epoch_in on first send and on a continuation, the fail-closed readers, and the job's incomplete-delivery retry.
brianjohnson5972
left a comment
There was a problem hiding this comment.
Peer review alongside wire-ethereum#216 and wire-tools-ts#111. The findings are inline. Overall: changes requested.
decide_delivery reproduces the contract's admission ladder exactly. Nonce handling, the ordering of estimate before floor, uint256 floor arithmetic, enum and logging handling, and the removed chunk paths are all correct. The issues cluster around the new gas floor:
- Major: it is never simulated at the gas limit actually sent, which pairs with the contract issue raised on #216 to make a permanent paid revert loop.
- Major: it ignores
max_total_native_cost. - Major: progress is read at
lateststraight after a 1-confirmation receipt.
Merge order / release: wire-ethereum#216 first or together with this. Once this is on master, the cranker can't drive the already-deployed hoodi/mainnet OPPInbound, so the fresh OPP redeployment is a release gate.
This was a static review; I ran no sysio build locally. CI is green.
| if (gas_limit_floor != 0) { | ||
| // The floor is bounded by the same ceiling as the estimate: a caller | ||
| // asking for more than the policy allows is refused, not clamped, so | ||
| // an under-funded call is never silently sent. | ||
| const fc::uint256 floor{gas_limit_floor}; | ||
| if (floor > _transaction_policy.max_gas_limit) { | ||
| throw_transaction_policy_exception(ethereum_transaction_policy_reason::gas_limit_cap_exceeded, | ||
| transaction_policy_field::gas_limit_floor, | ||
| floor.str(), | ||
| _transaction_policy.max_gas_limit.str()); | ||
| } | ||
| if (gas_limit < floor) gas_limit = floor; | ||
| } |
There was a problem hiding this comment.
[Major] The transaction is sent at a gas limit it was never simulated at, so a revert the estimate would have caught becomes a paid revert that repeats every tick.
eth_estimateGas proves the call succeeds at the estimate, but the transaction goes out at the floor (~16.7M). Against wire-ethereum#216 those two gas limits take different paths:
OPPInbound._dispatchFromstops only whengasleft() < 1M.- It hands each handler
gasleft*7/8. - A handler that runs out of gas reverts the whole transaction (
OPP_HandlerGasExhausted).
Failure scenario:
- At the estimate's low gas, the loop dispatches the forced first attestation, then spills cleanly, so the estimate succeeds.
- At 16.7M the loop runs further and reaches attestation k with ~1.2M left. That handler needs 1.1M but gets a 1.05M bound, so the transaction reverts and burns ~16.7M gas.
- On-chain state is unchanged, so the next tick (15 s) repeats the same estimate, floor and revert — forever.
Node dependence: geth post-Osaka estimates at its upper bound first and might catch this when that bound is exactly 2^24. anvil, hardhat and other clients estimate at the block gas limit and won't.
The contract-side half (spill instead of reverting when a mid-call handler runs out of gas) is raised on wire-ethereum#216 at OPPInbound.sol:1181. Both halves are needed.
Suggested fix: after raising to the floor, eth_call at exactly the gas_limit that will be sent. If that reverts, fall back to the buffered estimate, which is known to succeed and still makes progress at one attestation per transaction. Add a test where the estimate succeeds but execution at the floor reverts.
There was a problem hiding this comment.
The limit sent is the limit simulated now: cap = limit, estimate runs under it. 8caeb85.
| auto data = contract_encode_data(contract, params); | ||
|
|
||
| auto estimated_gas = estimate_gas(to, contract, data, gc); | ||
| auto gas_limit = derive_buffered_gas_limit(_transaction_policy, estimated_gas); |
There was a problem hiding this comment.
[Minor] A buffered estimate above the ceiling is rejected even when the floor would fund the call.
derive_buffered_gas_limit throws on estimate × 1.2 > max_gas_limit before the floor is considered. With the recommended max_gas_limit = 2^24, an epochIn estimate between ~13.98M and 16.7M (a heavy forced-first handler, say) is refused, even though the floored 16.7M transaction would run.
Suggested fix: when gas_limit_floor != 0, require estimate <= floor and use gas_limit = max(floor, …) without the 1.2× buffer check.
There was a problem hiding this comment.
Gone with the buffer: a capped call that fits the cap is sent with the cap.
| inline uint64_t delivery_gas_limit_floor(const ethereum_transaction_policy& policy) { | ||
| const fc::uint256 cap{EIP_7825_TX_GAS_CAP}; | ||
| return policy.max_gas_limit < cap ? policy.max_gas_limit.convert_to<uint64_t>() : EIP_7825_TX_GAS_CAP; | ||
| } | ||
|
|
There was a problem hiding this comment.
[Major] The floor ignores max_total_native_cost, so every delivery is priced at 16.7M gas.
validate_transaction_against_policy requires gas_limit × max_fee_per_gas <= max_total_native_cost (ethereum_transaction_policy.cpp:~373). Before this PR a delivery with a ~1–2M estimate fit a modest budget. Now every epochIn is checked at 16.7M × maxFee — including non-tipping first deliveries and no-op re-deliveries.
Consequences:
- With
max_gas_limit = 16.7Mandmax_total_native_cost = 1 ETH, every delivery is a policy rejection before signing whenever2·base + tipexceeds about 60 gwei. The rejection repeats every tick, so that operator never delivers. - Independent of the policy, the relay must hold about 16.7M × maxFee of upfront balance per transaction (around 1 ETH at a 30 gwei base fee), or the node rejects it with "insufficient funds".
docs/ethereum-client-config.example.jsonsetsmax_gas_limitto 2,000,000, which caps the floor at 2M. A full-cap 32 KiB first delivery needs more than that (calldata plus decode is about 9% of the cap at 31 KiB per the contract's own measurements), so it is undeliverable under the documented example.
Suggested fix:
- In
create_default_tx, clamp the floor tomax_total_native_cost / gc.max_fee_per_gas, never below the buffered estimate. - Document the balance requirement.
- Update the example config.
- Add a test with a finite total-cost policy.
Merge order: this PR's cranker only speaks epochIn(uint32,bytes). That ABI exists only once wire-ethereum#216 lands on next, so #216 needs to merge first or together with this one. Once this is on sysio master, the cranker can't drive the already-deployed hoodi/mainnet OPPInbound. Please record the fresh OPP redeployment as a release gate for the sysio release that carries this change.
There was a problem hiding this comment.
delivery_gas_ceiling(policy, max_fee_per_gas) bounds the ceiling by max_total_native_cost / fee at the current fee, so the budget is never a signing-time rejection. Example config updated, balance requirement documented, finite total-cost test added.
| const auto next_epoch = read_next_epoch_index(); | ||
| const auto spill = read_dispatch_spill(epoch_index); | ||
| // The two settlement reads are only meaningful — and only paid for — | ||
| // when the epoch is mid-flight. | ||
| const bool settled = spill.tipped && !spill.finalized && own_delivery_settled(epoch_index); |
There was a problem hiding this comment.
[Major] Progress is read at latest right after a 1-confirmation receipt, so a stale read can make the tipper abandon its own spill.
confirmations defaults to 1. nextEpochIndex, dispatchSpill, epochDeliveries and pendingEpochHash are then four independent latest calls, possibly served by different backends behind a load-balanced RPC. Two ways this goes wrong:
- Our own
delivertips and spills, but the next read comes from a lagging backend and still shows "not tipped".delivered_this_tickthen returnslast_tx(line 493) and the job marks the epoch handled. - The spill read shows tipped, but
epochDeliveries(self)is stale (zero). That yieldswait_for_deliverer, which also returns normally.
Either way the operator holding the continuation right stops. Finishing the epoch then falls to the job's single post-boundary retry (outpost_opp_job.cpp:86-89), which delays finalization by up to an epoch. If that retry also returns normally short of finalization, nobody continues.
Suggested fix: after a sent transaction, read progress pinned to the receipt's block number (block_number_or_tag_t accepts numbers). Better still, decide tipped / spilled / finalized from the receipt's EpochConsensus / EpochDispatchProgressed / EpochComplete logs.
There was a problem hiding this comment.
epoch_in returns the receipt; progress reads are pinned to its block, and the receipt's events are summarised in the log line.
| wlog("outpost_ethereum_client[{}]: epoch={} still not finalized after {} continuations in " | ||
| "one tick; the next tick resumes from the outpost's cursor", | ||
| to_string(), epoch_index, MAX_CONTINUATIONS_PER_TICK); | ||
| return last_tx; |
There was a problem hiding this comment.
[Minor] Running out of MAX_CONTINUATIONS_PER_TICK, and the wait_for_deliverer branch, both return normally, so the job treats the epoch as handled and this operator never resumes it.
On a normal return the job sets _last_outbound_epoch / _last_consensus_retry_epoch and never calls again for that epoch. A deadline expiry, by contrast, throws and is retried correctly. So the "the next tick resumes" in this wlog is not true for this operator: the epoch is permanently handed to other operators' retries.
Also, the loop's last iteration sends a transaction and exits without re-reading, so this warning can fire even when that transaction finalized the epoch.
Suggested fix: throw a typed "continuation budget exhausted" exception so the job retries next tick, and re-read once before warning. The same applies to wait_for_deliverer at line 486 when the read may be stale (see the comment at 464).
There was a problem hiding this comment.
Bound exhaustion throws the typed exception after a final read; wait_for_deliverer is decided from a pinned read, so staleness is no longer the concern there.
| const auto delivered = normalized_word(decoded_scalar(eth::contract_decode_data( | ||
| _opp_inbound_client->get_abi(opp_inbound_abi::view_epoch_deliveries), delivered_raw.as_string()))); | ||
| const auto settled = normalized_word(decoded_scalar(eth::contract_decode_data( | ||
| _opp_inbound_client->get_abi(opp_inbound_abi::view_pending_epoch_hash), settled_raw.as_string()))); | ||
| dlog("outpost_ethereum_client[{}]: epoch={} own delivery={} settled={}", | ||
| to_string(), epoch_index, delivered, settled); | ||
|
|
||
| // A zero digest is "nothing recorded" on either side and never a match: an | ||
| // epoch cannot settle on the zero digest, and a relay that has not | ||
| // delivered has no claim on the continuation. | ||
| if (is_zero_word(delivered) || is_zero_word(settled)) return false; |
There was a problem hiding this comment.
[Minor] own_delivery_settled fails open where its sibling reads fail closed.
read_dispatch_spill asserts that every field parses. Here, a word that doesn't decode becomes "", which is_zero_word treats as zero, so this returns false. That leads to wait_for_deliverer, an empty or normal return, and the epoch silently marked handled. A decoder or ABI shape change would therefore quietly stop every relay from continuing.
Suggested fix: FC_ASSERT that both decoded values are 32-byte hex words. Also consider checking keccak256(envelope_bytes) == settled locally before sending a continuation. Today a digest mismatch only surfaces as an OPP_DigestMismatch estimate revert, retried every tick under a generic warning.
There was a problem hiding this comment.
Both words asserted 32-byte; local keccak checked against the settled digest before any continuation.
|
|
||
| // Not tipped: deliver, whatever this relay's own record says. | ||
| BOOST_CHECK(detail::decide_delivery(test_wire_epoch, test_wire_epoch, untipped, false) == action::deliver); | ||
| BOOST_CHECK(detail::decide_delivery(test_wire_epoch, test_wire_epoch, untipped, true) == action::deliver); |
There was a problem hiding this comment.
[Minor] The new risk surfaces aren't tested.
delivery_decision_table covers the state × settled table well. Missing:
- running out of
MAX_CONTINUATIONS_PER_TICK, and what the job does next; - stale or inconsistent progress reads (see the comment at
outpost_ethereum_client.cpp:464); - a zero
pendingEpochHash, and an undecodableepochDeliveriesword; - that the real
epoch_inwrapper carriesdelivery_confirm_options, i.e. that a sent transaction'sgas_limit == min(max_gas_limit, 2^24). The delivery fixture replacesepoch_inat the typed-callable boundary, so nothing proves this today. A recording client would. - the floor against
max_total_native_cost, and the case where the floor execution reverts but the estimate succeeds (see the libfc comment).
There was a problem hiding this comment.
Covered: bound exhaustion plus job retry, stale reads (pinned, and re-read after stale refusals), zero and undecodable digests, the wrapper's budget reaching the estimate, the finite total-cost ceiling. "Floor reverts but estimate succeeds" is moot now that the simulated limit is the sent limit.
| } | ||
| ], | ||
| "name": "isActiveOperator", | ||
| "name": "isActiveOperatorForEpoch", | ||
| "outputs": [ | ||
| { | ||
| "internalType": "bool", |
There was a problem hiding this comment.
Nit: this fixture is behind wire-ethereum#216's head (032e6162). It still has lastMessageID, pendingEpoch and pendingMessageCount, which that commit removed. It is also missing installInitialRoster, FIRST_INBOUND_EPOCH_INDEX and OPP_BootstrapWindowClosed, and the OPP fixture is likewise missing OPP_BootstrapWindowClosed.
Every entry the cranker actually binds matches exactly:
epochIn(uint32,bytes)dispatchSpill(uint32)epochDeliveriespendingEpochHashnextEpochIndexattestationHandlers
Please regenerate both fixtures from the #216 head before merging.
There was a problem hiding this comment.
Both fixtures regenerated from the current artifacts.
| /// ours to clear. Hashed rather than written out as its selector so a reader can check this | ||
| /// against the ABI directly; `chunk_buffer_missing_selector_is_pinned` holds the hash to the | ||
| /// four bytes the contract actually emits. |
There was a problem hiding this comment.
Nit — chunk-era leftovers:
- lines 40-43: an orphaned doc comment for the removed
discardEnvelopeChunksselector constant; - line 54: a reference to
ETHEREUM_MAX_CHUNK_BYTES; - line 256 and test
:1402: references toOPP_ChunkBufferMissing; outpost_opp_job.cpp:~121: "chunked Ethereum deliveries";plugins/outpost_ethereum_client_plugin/README.md:100-125,266(not in this diff): still documentsepochIn(uint32,uint16,uint16,uint32,bytes),discardEnvelopeChunks,envelopeChunkStateand the chunk resume table.
There was a problem hiding this comment.
All removed; README sections rewritten.
| // SIZING RULE for batch-delivery-timeout-ms: it bounds the WHOLE outbound | ||
| // delivery, and an Ethereum delivery is now one transaction PER CHUNK | ||
| // (ETHEREUM_MAX_CHUNK_BYTES = 8192; Solana chunks at 672). Size it as | ||
| // (whole-envelope on Ethereum, continued while it spills; Solana chunks at 672). Size it as |
There was a problem hiding this comment.
Nit: only one line of this sizing rule was updated. It still says "one transaction PER CHUNK", "4 Ethereum chunks", and that the relay resumes "from the on-chain per-operator high-water mark". Under whole-envelope delivery the rule is "one delivery plus N continuations, each ~one block plus confirmation", and the resume point is the outpost's dispatchSpill cursor.
Pre-existing, outside this diff, but it limits liveness here: the job's single consensus retry is gated on the depot's next_epoch_start, while the outpost's path-2 boundary is currentEpochStartedAt + epochDurationSec, which lags it. The one retry can therefore fire before the outpost boundary, re-deliver without tipping, and be used up. Worth a follow-up.
There was a problem hiding this comment.
Comment rewritten. The retry gate: the relay reads the outpost's own boundary and keeps the epoch open until it passes, so the single retry can't fire into a no-op.
b9263af to
e45ead6
Compare
`create_default_tx` sizes every transaction from `eth_estimateGas` plus a buffer. That is right for a call that either does its work or reverts, and wrong for one that stops on a `gasleft()` watchdog and records where it stopped: the estimator converges on the least gas at which such a call SUCCEEDS — the smallest amount of work it can do and still return cleanly — so the estimate systematically under-funds it. `ethereum_confirm_options::gas_limit_floor` lets the caller of a `create_tx_and_confirm` wrapper fund the call to a floor instead. The floor is applied after the buffered estimate and is subject to the same policy ceiling: a floor above `max_gas_limit` is a policy rejection (`gas_limit_cap_exceeded`, field `gas_limit_floor`), never a silently clamped limit. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Change-Id: I72d029cda7d08eb19b921a971af7af8c7f4c922a
…om the outpost's cursor The Ethereum outpost no longer takes an envelope in 8 KiB chunks staged on chain. `OPPInbound.epochIn(uint32 epochIndex, bytes envelopeData)` takes the WHOLE envelope in one call — Ethereum bounds a transaction by gas, not size, and a full-cap envelope is ~1.3 M gas of calldata — and what may not fit is dispatch, which the contract spills across continuations of the same call. The outpost never stores the bytes; every continuation re-supplies them. THE RELAY DELIVERS OR CONTINUES. Each tick reads the outpost's own cursor (`nextEpochIndex`, `dispatchSpill(epoch)`) at `latest` and decides: - finalized, or the epoch cursor is past it -> nothing to send - tipped on the digest THIS relay delivered -> re-supply the envelope - tipped on a digest it did not deliver -> its deliverers continue - not tipped -> deliver `decide_delivery` is the pure table; the loop around it continues a spilling epoch within the tick until the outpost reports it finalized, bounded by the deadline and `MAX_CONTINUATIONS_PER_TICK`. A delivery that records without tipping stops the tick rather than re-sending a paid no-op; the job's boundary-gated retry still covers path-2 consensus. Whether this relay's delivery is the settled one is read from `epochDeliveries(epoch, self)` against `pendingEpochHash()`, because the contract admits a continuation only from a deliverer of the settled digest. FUNDED TO THE CEILING, NOT THE ESTIMATE. `epochIn` stops dispatching on a `gasleft()` watchdog instead of reverting, so `eth_estimateGas` converges on the tip plus ONE attestation and would spill every call after one attestation. The wrapper is built with `gas_limit_floor` = the client's policy ceiling bounded by EIP-7825's cap (`delivery_gas_limit_floor`), so one call carries as much dispatch as the chain allows; the unused remainder is refunded. Retired with the chunk loop: `ETHEREUM_MAX_CHUNK_BYTES`, `chunk_count_for`, the staging-header resume read, `discardEnvelopeChunks`. The ABI fixtures are refreshed from the rebuilt wire-ethereum artifacts (`epochIn(uint32,bytes)`, selector 004e356a), and the plugin tests exercise every branch of the decision table plus deadline abandonment and resumption against a stubbed OPPInbound. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Change-Id: I8543310904e320d13101aee4c7c2bca6936ffc73
The cranker funded every `epochIn` to the policy ceiling. A node reserves `gas_limit * max_fee_per_gas` of the signer's balance whatever the call uses, so a small envelope funded to the cap was refused for balance at exactly the fee levels where delivery matters most. The node's estimate is wrong the other way: the contract stops dispatching on a gas watchdog and records where it stopped, so `eth_estimateGas` converges on the tip plus one attestation and funding to it costs one transaction per attestation. `delivery_gas_budget` sizes each call from what it has to do: a fixed cost, a per-byte cost for calldata and the decode, an allowance per attestation the outpost has not dispatched yet, and the emit the finishing call attempts. A spill met earlier in the same tick doubles the next budget, so a run of attestations dearer than the allowance costs a logarithmic number of extra calls; the ceiling (policy max, bounded by EIP-7825) still caps it. An envelope the cranker cannot read is funded to the ceiling. libfc gains `ethereum_confirm_options::gas_limit_cap`: the transaction carries at most the cap, and `eth_estimateGas` is sent with `gas` set to it, so a call the budget cannot carry is refused by the node before anything is signed. The delivery options set floor and cap to the same budget. A JSON-RPC refusal of the pre-flight is retried once at the ceiling when the budget was below it. The epoch-in callback takes the budget per call, since a plain `ethereum_contract_tx_fn` binds its options once at construction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Change-Id: Iddd9d1349e000d7fe0133efdf2b053c3151e4612
… confirmed receipt `create_default_tx` with a `gas_limit_cap` now sends the transaction with exactly the cap. The pre-flight estimate still runs under it, so a call that cannot succeed inside the cap is refused by the node before anything is signed, but the estimate's VALUE is no longer an input to the limit: neither the x1.2 headroom buffer nor the buffered-estimate policy check applies, since what is simulated is exactly what is sent. Before, a capped call whose estimate used most of its cap was rejected by `derive_buffered_gas_limit` (buffered estimate above `max_gas_limit`) for a transaction the cap already carried -- the review's #13 on PR #660. `wait_for_receipt` returns the `eth_getTransactionReceipt` object after the same two-phase wait `wait_for_confirmation` performs (which now delegates to it). A caller deciding its next move from the chain reads the receipt's `blockNumber` and `logs` instead of re-reading state at `latest`, which a lagging backend may serve from before the transaction landed. Tests: the cap case now pins that a cap of 1200 is sent as 1200 (not the buffered 1000), that a capped call fits a policy its buffered estimate would breach, and that `wait_for_receipt` hands back the receipt's block and logs, honours the depth wait, and rejects a reverted receipt. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Change-Id: I855041d6ebb93bac73044faea674a509f46ecdbd
…er call, and a fee-bounded gas ceiling
The relay's `deliver_outbound_envelope` now takes every decision from reads
pinned to one block -- the head when the tick starts, then the block each
confirmed `epochIn` landed in (from its receipt) -- never `latest`, which a
backend behind the block that just confirmed could serve from before the
call. A backend that does not know the block yet is retried for a bounded
time. `epoch_in` returns an `epoch_in_receipt` (hash, block, logs) built from
`create_default_tx` + `execute_contract_tx_fn` + `wait_for_receipt`, and the
`OPPInbound` events in the receipt are summarised into the per-call log line.
Every call paid for must move the outpost's cursor (`advanced`): a confirmed
call that left every field where it was was under-funded for the attestation
at the cursor, so the next is funded double; one at the ceiling that still
moves nothing is logged once at error level per cursor position and ends the
tick in the new `outpost_delivery_incomplete_exception` (3110009), which the
job logs at info and does NOT mark handled, so the next tick resumes. The
same exception ends a tick that reaches `MAX_CONTINUATIONS_PER_TICK` with the
epoch still open (the loop no longer sends 33 calls and returns normally),
and a tick whose outpost is still on an earlier epoch (`outpost_behind`,
instead of a non-sequential revert). Before a continuation the relay checks
that `keccak256` of the envelope it holds IS the settled digest, and sends
nothing when it is not.
A recorded, untipped delivery is re-sent only when the outpost's own
`pendingConsensusForDigest(own digest)` says the contract's path-2 predicate
holds (boundary elapsed AND a strict majority agreeing); otherwise the relay
waits for the group. Settlement is a four-way classification
(`never_delivered` / `recorded` / `divergent` / `settled`) that asserts both
words are 32 bytes instead of failing open.
Gas: `delivery_gas_ceiling(policy, max_fee_per_gas)` bounds the ceiling by
what `max_total_native_cost` pays for at the fee the call is sent at, so a
budget the ceiling admits is never refused by the policy at signing. A client
handed an OPPInbound address is refused at construction when its static
ceiling is below `DELIVERY_MINIMUM_GAS_CEILING` (one full-cap delivery:
9 932 160 gas), and warned when its bounded total-cost term is. Estimate-time
reverts are classified: `OPP_DispatchUnderfunded` / `OPP_HandlerGasExhausted`
below the ceiling retry once at the ceiling; `OPP_NonSequentialEpoch`,
`OPP_OperatorAlreadyDelivered`, `OPP_NotActiveOperator` and
`OPP_DigestMismatch` re-read at the head a bounded number of times.
Also: chunk-era comments and the stale `discardEnvelopeChunks` /
`ETHEREUM_MAX_CHUNK_BYTES` mentions removed; README delivery, policy,
diagnostics and test sections rewritten; `docs/ethereum-client-config.example.json`
raised from a 2M `max_gas_limit` (below the emit floor) to the EIP-7825 cap;
the delivery-timeout sizing comment and the `outpost_client` SPI doc updated;
`tests/fixtures/ethereum-abi-opp{,-inbound}-current.json` regenerated from the
current wire-ethereum artifacts (hardhat JSON, 117 entries, incl. the events
and `pendingConsensusForDigest`).
Tests: the delivery fixture now runs over a scripted chain (block number, fee
RPCs) with receipt-returning `epochIn`, pinned-block view stubs and scripted
refusals. New cases: fee-bounded ceiling, refusal of an undeliverable policy,
settlement classification, the path-2 predicate, the progress invariant,
pinned revert selectors and event topics, receipt summaries, consensus retry
gated on the outpost's majority view, escalation after a no-op call and the
stall report at the ceiling, gas refusals retried at the ceiling, stale
refusals re-read at the head, outpost behind, the continuation bound, and a
continuation refused with a non-settled envelope. The job test covers the
incomplete-delivery retry.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Change-Id: I67e24428e4e82717e0f7a7025ce064826fe11e5a
e45ead6 to
7519ca4
Compare
Summary
The Ethereum outpost cranker for whole-envelope calldata delivery, matching wire-ethereum's
OPPInbound.epochIn(uint32,bytes). Stacked on master (post #662).libfc(ethereum): a confirmed write can be funded to agas_limit_floor, or sent with exactly agas_limit_cap. With a cap the pre-flighteth_estimateGasruns withgas= cap and its value is not an input to the limit (no ×1.2 buffer, no buffered-ceiling check): what is simulated is what is sent, so a call the cap cannot carry is refused by the node before anything is signed.wait_for_receiptreturns the confirmed receipt;wait_for_confirmationdelegates to it.outpost_ethereum_client: eachepochInis funded todelivery_gas_budget— a fixed cost, a per-byte cost, an allowance per attestation the outpost has not dispatched yet, and the emit — doubled per call this tick that fell short, never abovedelivery_gas_ceiling(policy, max_fee_per_gas)(policymax_gas_limit, EIP-7825's cap, andmax_total_native_cost / feeat the current fee). A client handed an OPPInbound address is refused at construction when its static ceiling cannot fund one full-cap delivery (DELIVERY_MINIMUM_GAS_CEILING, 9 932 160 gas) and warned when a bounded total-cost term cannot.latest.epoch_inreturns anepoch_in_receipt(hash, block, logs) and the receipt'sOPPInboundevents are summarised per call.advanced): a confirmed call that moved nothing is funded double next; one at the ceiling that still moves nothing is logged once per cursor position and ends the tick in the newoutpost_delivery_incomplete_exception(3110009), which the job logs at info and does not mark handled. The same exception ends a tick atMAX_CONTINUATIONS_PER_TICK(after a final read), one whose outpost is still on an earlier epoch (outpost_behind), and a consensus retry that arrives before the outpost's own boundary (so the job's single retry is not spent early).OPP_DispatchUnderfunded/OPP_HandlerGasExhaustedbelow the ceiling retry once at the ceiling;OPP_NonSequentialEpoch,OPP_OperatorAlreadyDelivered,OPP_NotActiveOperator,OPP_DigestMismatchre-read at the head, a bounded number of times.decide_delivery(next_epoch, epoch, spill, settlement, majority_reachable)yieldsdeliver | retry_consensus | await_peers | continue_dispatch | wait_for_deliverer | already_finalized | outpost_behind. Settlement is four-way (never_delivered | recorded | divergent | settled, 32-byte words asserted); a continuation is sent only whenkeccak256of the envelope held ISpendingEpochHash; a recorded, untipped delivery is re-sent only whenpendingConsensusForDigest(own digest)shows the contract's path-2 predicate holds. The inbound-envelope read stays atfinalized.--rpc.txfeecap; the example config funds a full delivery.test_outpost_ethereum_client_plugin(scripted chain + receipt-returningepochIn),test_batch_operator_pluginandtest_fc -t '*ethereum*'report no errors; ABI fixtures regenerated from the wire-ethereum Add missing COMPONENT to all install() rules for correct deb packaging #216 artifacts.Depends on the wire-ethereum PR (cross-linked below) for the contract ABI.
Companion PRs
E2E gate
e2e-tests run 37653921624 —
success, full platform build plus every discovered flow, dispatched withBRANCH_WIRE_SYSIO,BRANCH_WIRE_ETHEREUMandBRANCH_WIRE_TOOLS_TS=feature/eth_opp_calldata_delivery(wire-libraries-ts, wire-solana and wire-cdt on their manifest branches). Resolved revisions: wire-ethereumaa650ea, wire-sysioe45ead632a, wire-tools-ts27c21985. That run predates the review-fix commits (8caeb85167,7519ca41f9) and the rebase onto post-#662 master; a re-run on the current heads (with wire-ethereum #216 now stacked on #217) is pending.All 15 flows passed. The same 15 flows also pass locally on the same revisions.
🤖 Generated with Claude Code