feat(cketh): read a native ETH balance via the EVM RPC canister - #11060
Open
mbjorkqvist wants to merge 6 commits into
Open
feat(cketh): read a native ETH balance via the EVM RPC canister#11060mbjorkqvist wants to merge 6 commits into
mbjorkqvist wants to merge 6 commits into
Conversation
The EVM RPC canister exposes no `eth_getBalance` endpoint and `evm_rpc_client` offers no balance getter, so read the balance through the canister's generic `multi_request`: it forwards an arbitrary JSON-RPC payload to every provider, parses each response's `result` field and reduces those under the configured consensus strategy. Reduction on the parsed `result` rather than the raw body means providers' JSON-envelope differences are irrelevant; only the hex quantity is compared, with a strict majority required. Reading fails rather than defaulting to zero on any error: the first consumer is the sweeper address' ETH balance, which *is* the prepaid-sweep-gas counter counter, where confusing "no gas left" with "could not read the balance" would trigger a spurious ckETH burn. Request building and result decoding are pure functions so they are unit tested directly: payload shape, lowercase address rendering, every named block tag, minimal-length block-number quantities, and decoding of valid quantities including zero and the 32-byte maximum. Rejection is asserted for non-quantities, values wider than 32 bytes, and — as its own test — that no error input can ever decode to a zero balance. The whole route is additionally proven end to end against a live EVM RPC canister and a local anvil node on the DEFI-2933 spike branch, reaching 3-of-4 consensus at both `latest` and `finalized`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds ckETH minter support for reading a native ETH balance (eth_getBalance) via the EVM RPC canister’s generic JSON-RPC passthrough, including request construction and robust decoding with unit tests to ensure errors never decode as a zero balance.
Changes:
- Exposes a new
eth_rpc_client::get_balancemodule. - Implements
eth_get_balance+ helpers to build the JSON-RPC payload and decode the hex-quantity result intoWei. - Adds unit tests covering request formatting, block tag rendering, and decoding/validation behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rs/ethereum/cketh/minter/src/eth_rpc_client/mod.rs | Exposes the new get_balance module from the eth RPC client. |
| rs/ethereum/cketh/minter/src/eth_rpc_client/get_balance.rs | Implements request building, strict-majority reduction handling, and decoding for eth_getBalance. |
| rs/ethereum/cketh/minter/src/eth_rpc_client/get_balance/tests.rs | Adds unit tests for request encoding and balance decoding invariants. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Point the design-doc reference at its repo-relative path; `docs/…` resolves relative to nothing a reader can follow, since there is no `docs/` beside the crate. Matches the existing reference in `balance_scan/batcher`. - Correct a comment that claimed the hex digits had already been validated before `from_str_hex` is called. The check above establishes only the `0x` prefix — which is why the closure re-checks the digits at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (1)
rs/ethereum/cketh/minter/src/eth_rpc_client/get_balance.rs:62
StrictMajorityByKeyis currently keyed on the rawStringreturned by providers (balance.clone()), which makes consensus sensitive to harmless formatting differences in the hex quantity (e.g. uppercase vs lowercase, leading zeros, or quoted results). This contradicts the goal of comparing only the quantity value and can cause false “no consensus” errors even thoughdecode_balanceexplicitly tolerates these forms.
// A balance is compared for exact equality across providers: at a finalized block every
// honest provider must return the same quantity, so anything else is a disagreement
// worth surfacing rather than papering over by picking one answer.
.reduce_with_strategy(StrictMajorityByKey::new(|balance: &String| balance.clone()))
.map_err(GetBalanceError::Rpc)?;
The strict-majority reduction keyed on the raw string a provider returned, so two providers reporting the same balance in different renderings — differing only in hex case, say — counted as a disagreement and failed the read. That fails safe (never a wrong balance) but stalls funding for no reason, and it contradicts the module's claim that only the quantity is compared. Key on the decoded amount instead. An answer that does not decode keeps its raw form as the key, so distinct garbage still disagrees rather than collapsing into a false majority. The key is a named function so the property is unit-testable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two problems with reading the sweeper's balance, both of which could turn a bad provider answer into a burn. The reduction accepted a plurality, not a majority. `StrictMajorityByKey` returns the largest ballot whenever it beats the runner-up, so a 2/1/1 split across four providers wins on two votes. Worse, it only ever runs on results the EVM RPC canister has already declared inconsistent — i.e. exactly when the configured threshold (3-of-4 on mainnet) was not met — so it was salvaging an answer from the case the threshold exists to reject. Drop the client-side reduction: the canister's consensus is the policy, and a disagreement stays an error. The earlier by-value comparison goes with it, since there is no longer a key to compare on; the tolerant renderings it covered are still exercised against the decoder. The decoder could also read malformed input as a balance of zero, which is the expensive direction: zero means "the sweeper is empty" and buys a burn that was never needed. Stripping quote characters accepted one-sided and repeated quotes, so `"0x0` decoded to zero; the quoted form is now parsed as JSON. Non-minimal quantities like `0x00` were accepted too, and are now rejected as not being the protocol's. Both cases join the test that pins "an error never decodes to zero". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
rs/ethereum/cketh/minter/src/eth_rpc_client/get_balance.rs:108
- Validate
digitsbefore callingWei::from_str_hex. The underlying unsigned integer parser accepts an optional+, so a malformed quantity such as0x+0can currently parse asWei::ZERO; theis_ascii_hexdigitcheck only runs after parse failures and therefore does not catch it.
Wei::from_str_hex(&quantity).map_err(|_| {
// `from_str_hex` rejects both non-hex digits and values wider than 32 bytes, and the
// checks above established only the shape of the quantity — so tell the two apart here.
if digits.chars().all(|c| c.is_ascii_hexdigit()) {
`multi_request` deserializes each response's `result` field into a string (`RawJson` is `#[serde(transparent)]` over `String`), so what reaches the decoder is the quantity itself — no envelope, no quotes, no padding. Trimming whitespace and unquoting therefore did not tolerate a caller's raw JSON, as its comment claimed; it repaired malformed provider answers. Both ` 0x0 ` and `"0x0"` decoded to zero, and zero is the expensive value to get wrong: it reads as "the sweeper is empty" and buys a burn that was never needed. Decode exactly, and validate the digits before parsing rather than inferring them from a parse failure. That also puts `from_str_hex`'s tolerance for a leading `+` out of reach, and leaves "too large" as the only rejection the parser itself can still produce. The module doc claimed the reduction compared only the hex quantity; with the client-side reduction gone that sentence described nothing, so it now says what the canister does and what this module accepts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The consensus threshold is `rpc_client`'s and varies by network — 3 of 4 providers on mainnet, 2 of 4 on Sepolia — so the guarantee is "the configured threshold agreed", not "a majority agreed". Both the module doc and the comment at the call site said "threshold" without saying what it is, which invited the stronger reading. Keeping the shared threshold is deliberate rather than an oversight: holding this one read to a stricter bar would need its own client, and would buy nothing on the only network where the numbers differ, since ckSepoliaETH is not worth anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
✅ No security or compliance issues detected. Reviewed everything up to 451a521. Security Overview
Detected Code Changes
|
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.
Part of DEFI-2933 (sweeper fee funding), first of a seven-PR stack.
Why
Funding the sweeper address with gas requires knowing how much gas it already holds. The EVM RPC canister exposes no endpoint for a native ETH balance, and its Rust client offers no getter for one, so the minter currently has no way to ask.
What
Reads the balance through the EVM RPC canister's generic JSON-RPC passthrough, which forwards a payload to every provider and agrees on one answer under the configured consensus strategy. That strategy is a threshold of the providers — 3 of 4 on mainnet, 2 of 4 on Sepolia — and it is the only agreement accepted: there is no client-side reduction, so a result the canister reports as inconsistent stays an error rather than being resolved by picking a winner.
Because the canister deserializes each response's
resultfield, what the minter receives is the quantity itself rather than any surrounding JSON. It is therefore decoded exactly: quotes, padding, leading zeros and sign characters are the provider's own malformation and are rejected rather than repaired.A failed read is an error, never a zero. This is the decision the rest of the stack depends on: confusing "could not read the balance" with "no gas left" would burn ckETH to top up an address that is already funded, which is pure loss. The request builder and the result decoder are pure functions so both sides of that guarantee are pinned directly, including a test asserting that no error input can decode to a zero balance.
The route was also proven end to end against a live EVM RPC canister and a local anvil node, reaching 3-of-4 consensus at both
latestandfinalized.Stack
Merge in order; each PR targets the one above it.