Skip to content

feat(zetaclient): emergency TSS native-fund drain (EVM + BTC) - #4612

Open
kingpinXD wants to merge 24 commits into
mainfrom
feat/tss-emergency-drain
Open

feat(zetaclient): emergency TSS native-fund drain (EVM + BTC)#4612
kingpinXD wants to merge 24 commits into
mainfrom
feat/tss-emergency-drain

Conversation

@kingpinXD

@kingpinXD kingpinXD commented Jul 21, 2026

Copy link
Copy Markdown
Member

Summary

  • Adds a zetaclient-only way to drain all native TSS funds — every EVM chain's gas token plus BTC — to a safe wallet for a crosschain shutdown. No zetacore changes.
  • An off-chain, signed JSON payload (served by zetatool drain-payload --serve, draft → freeze → final) pins the fully-resolved transaction for each chain. Every zetaclient verifies it against a baked-in key and, at the trigger height, TSS-signs and broadcasts. The receiver is hardcoded per network as an anti-redirect anchor.
  • BTC sweeps spend only the payload-pinned UTXOs (≤20 inputs each), signed through the existing SignTx path.
  • Gated behind a drain build tag, off by default; fails closed if the pubkey or receiver isn't configured.
  • Design, coordination model, and security notes: docs/rfc/002_tss_emergency_drain.md.
  • Verified end to end: a real 2-node localnet drains both EVM and BTC (drain_tss).

Before merge / any real drain (operator): fill OperatorPubKeyHex and the testnet/mainnet receiver addresses — both stay fail-closed sentinels until set.

🤖 Generated with Claude Code


Note

High Risk
Moves all native TSS funds via TSS signing with security relying on compile-time receivers, operator signatures, and optional drain builds—misconfiguration or a bad release could redirect or prematurely sweep funds.

Overview
Adds an emergency shutdown path to sweep native TSS balances on external EVM chains and Bitcoin to network-hardcoded safe wallets, without zetacore changes.

Operators use zetatool drain-payload to build operator-signed JSON from live balances, gas, nonces, and pinned UTXOs, optionally served via a draft → freeze → final HTTP cron. zetaclientd with the drain build tag arms only when ZETACLIENT_DRAIN_URL is set: it polls, verifies the baked-in operator key, enforces receiver anchors, and at the trigger Zeta height TSS-signs and broadcasts pinned EVM transfers and BTC sweeps (≤20 inputs per tx).

Shared fee math lives in pkg/migration (EVM migration CCTX now calls ComputeEVMMigration). Localnet CI adds start-drain-test, drain/drain_localnet image tags, and drain_tss e2e. Testnet/mainnet pubkey and receivers remain UNSET until configured before any real drain.

Reviewed by Cursor Bugbot for commit 49e2cd2. Configure here.

Greptile Summary

Adds a build-tag-gated emergency mechanism for moving native TSS funds to network-specific receiver anchors.

  • Introduces signed, canonical drain payloads and a draft-to-final payload server.
  • Generates pinned EVM transfers and batched Bitcoin sweeps from live chain state.
  • Adds a zetaclient poller that verifies payload signatures, trigger windows, receivers, fees, and signer availability before TSS signing and broadcasting.
  • Adds localnet orchestration, end-to-end coverage, build configuration, and operational documentation.

Confidence Score: 5/5

The PR appears safe to merge based on the reviewed drain generation, verification, signing, and broadcast paths.

The drain remains disabled outside tagged and explicitly armed builds, validates signed payloads against fixed receiver anchors, and constrains transaction construction and execution without an identified actionable failure.

Important Files Changed

Filename Overview
zetaclient/drain/poller.go Implements signed-payload polling, trigger-window coordination, receiver and fee validation, retry tracking, and concurrent EVM/BTC execution.
cmd/zetaclientd/drain_on.go Arms the drain only for tagged builds with validated network anchors, operator key, Bitcoin address, and live signer resolvers.
cmd/zetatool/cli/drain.go Adds payload generation and serving from live TSS addresses, EVM state, Bitcoin UTXOs, filters, and operator-supplied timing.
pkg/drain/generate.go Deterministically computes EVM drain transactions and partitions economical Bitcoin sweeps into bounded input groups.
pkg/draintx/payload.go Defines the drain transaction schema and deterministic signed encoding covering all execution-relevant fields.
zetaclient/chains/bitcoin/signer/sign.go Exposes the existing TSS Bitcoin transaction-signing path for the newly constructed pinned sweeps.
zetaclient/chains/evm/signer/sign_drain.go Adds deterministic signing and broadcasting for payload-pinned native EVM transfers.

Sequence Diagram

sequenceDiagram
    participant O as Operator / zetatool
    participant S as Payload server
    participant Z as Zetaclient poller
    participant C as Zetacore height provider
    participant T as TSS signers
    participant E as External chains
    O->>O: Resolve balances, nonces, gas, and UTXOs
    O->>O: Build and sign final payload
    O->>S: Publish final payload
    loop Poll interval
        Z->>S: Fetch payload
        Z->>Z: Verify signature and receiver anchors
        Z->>C: Read current Zeta height
    end
    alt Inside trigger window and signers ready
        Z->>T: Sign pinned EVM and BTC transactions
        T-->>Z: Signed transactions
        Z->>E: Broadcast transactions
    else Not ready or outside window
        Z->>Z: Wait, retry, or reject payload
    end
Loading

Reviews (1): Last reviewed commit: "chore: fix changelog, formatting, and li..." | Re-trigger Greptile

kingpinXD and others added 14 commits July 20, 2026 18:21
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move the EVM migration fee/amount math into pkg/migration.ComputeEVMMigration
so the on-chain CCTX builder and the upcoming drain tool share one source of
truth for the gas multiplier and buffer constants. Add ComputeBTCMigration for
the sweep fee/output math. No behavior change on the existing migration path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deterministic length-prefixed canonical encoding (signature excluded), keccak256
digest, and secp256k1 Sign/Verify via go-ethereum crypto. Verify accepts both
compressed and uncompressed baked-in pubkeys.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add pkg/drain, the network-agnostic generator that partitions UTXOs into disjoint
<=20-input sweeps and computes per-tx fees, plus the shared hardcoded per-network
receiver map. Wire a zetatool 'drain-payload' subcommand that derives balances,
median gas, nonces and UTXOs from the configured RPCs and emits a signed payload.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
New zetaclient/drain package: polls a signed payload, verifies against a baked-in
pubkey, enforces the compiled-in safe receiver, gates on the [H, H+window) firing
window, and drives EVM + BTC TSS signing at the pinned height. EVM signs via a new
build-tagged SignDrainTx that drives the keysign directly (the batch loop needs a
CCTX we don't have); BTC builds a wire.MsgTx from pinned inputs and reuses SignTx.

All drain code is behind the 'drain' build tag and off unless armed via
ZETACLIENT_DRAIN_URL, so production builds are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PayloadServer serves the latest payload over HTTP (the no-S3 local variant the e2e
uses); Publish swaps atomically so a final supersedes drafts. RunCron republishes
drafts on an interval then publishes exactly one final and stops.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drain-only e2e (no keygen, no MsgUpdateTssAddress): disables inbound, builds and
serves a signed final payload sweeping native TSS funds to the compiled localnet
receivers, waits for the txs to mine, and asserts TSS balances drain to ~0.

Runs only when the localnet zetaclients are built with -tags drain and armed via
ZETACLIENT_DRAIN_URL + ZETACLIENT_DRAIN_SIGNING_KEY; skips cleanly otherwise.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Makefile: overridable BUILD_TAGS; localnet zetanode image built with the drain
  tag (LOCALNET_BUILD_TAGS); new start-drain-test target that arms the clients.
- Dockerfile-localnet: forward BUILD_TAGS to make install.
- pkg/drain: ResolveAnchors with localnet-only env overrides for pubkey + receivers
  (testnet/mainnet keep the compiled anchors); real throwaway localnet EVM receiver.
- drain_on.go: use ResolveAnchors; env-overridable firing window.
- zetae2e local: --drain-test flag runs the drain test in isolation after setup.
- docker-compose: arm zetaclient0/1 + orchestrator with drain env.
- test_drain_tss.go: self-fund the TSS (ETH + BTC UTXOs) then drain and assert.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ron CLI

MAJOR: fail-closed receiver guards + UNSET sentinels for testnet/mainnet, non-zero
localnet receiver, arm-time pubkey/receiver validation; e2e injects a fresh non-zero
receiver. Wire the draft->freeze->final cron into 'zetatool drain-payload --serve'
with a height-aware freeze at H-K.

MEDIUM: poller resolves signers live (waits for EVM+BTC coverage); execute fires
per-tx ceremonies concurrently and retries failed txs across [H,H+window) instead of
marking done on first attempt; validate BTC fee (Σinputs-output==fee, bounded) before
signing.

MINOR: BTC dust floor in GenerateBTCTxs; generator warns to stderr on skipped chains
+ prints included summary; doc fixes (ComputeBTCMigration drain-only, GetEVMNonce
latest, receivers comment); per-family summary log after execute.

TESTS: EVM digest determinism; buildSweep determinism; validateBTCFee; step
fire/complete + context-cancel; zero-receiver reject; cli latestTSS/pickMedian/
drainNetwork; receivers.Validate + localnet-only env override; arm-time pubkey/window.

RFC updated for the shared-formula and cron drift.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… btc setup)

--skip-regular skips the bitcoin test setup that matures coinbase to the deployer
wallet, so the self-funding SendToTSSWithMemo had 0 spendable BTC. Mine 101 blocks
to the deployer address at the start of fundTSS.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A stray zetaclientd build artifact was previously committed; ignore root binaries
so it can't recur.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

!!!WARNING!!!
nosec detected in the following files: pkg/draintx/payload.go, pkg/migration/migration.go, zetaclient/drain/poller.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

@github-actions github-actions Bot added the nosec label Jul 21, 2026
Comment thread cmd/zetatool/cli/drain.go
Comment thread cmd/zetatool/cli/drain.go
Comment thread pkg/drain/server.go
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Comment thread cmd/zetaclientd/drain_on.go Outdated
kingpinXD and others added 4 commits July 21, 2026 00:15
…ests

Share the BTC fee/overhead (conservative fee + RBF reserve + nonce-mark
buffer) between tss-balances and the drain sweep via migration.ComputeBTCMigration,
so the drained amount matches what tss-balances reports. Add a mempool.space
UTXO fetch and env-gated live-RPC tests that build the EVM/BTC outbound txs
from real testnet data for inspection.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The BTC sweep charged the full migration overhead (miner fee + 0.01 BTC RBF
reserve + nonce-mark buffer) per 20-UTXO group. On a real TSS with hundreds
of UTXOs that multiplied the reserve across every tx (~0.27 BTC on mainnet)
and aborted the whole drain when a dust group could not cover its overhead.

Add migration.ComputeBTCSweep (miner fee only) for the multi-tx sweep, keeping
ComputeBTCMigration's full overhead for the single-tx tss-balances display.
Sort UTXOs by amount desc (deterministic, tie-broken by txid/vout) so value
concentrates into economical txs, and skip groups that cannot cover the fee
instead of aborting. Make the live test network-selectable so BTC can be
inspected against real mainnet data.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ron immediately

Address PR review:
- buildBTCTxs used the first Vm_no_vm chain from SupportedChains, which can differ
  from the BTC chain the TSS address/UTXOs are keyed to; use btcChainID directly so
  BTC isn't left undrained on a mismatch.
- generate() refuses to sign an empty final payload (0 EVM + 0 BTC), which would let
  the poller "succeed" while moving nothing. Drafts and subset payloads still allowed.
- RunCron publishes once immediately before the ticker so clients aren't blocked a full
  interval and the single final can't be delayed a tick; add coverage.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…chain subset

The poller no longer exits after one payload. It fires per final payload whose trigger
height is newer than the last one it handled (tracked via lastFiredHeight), so an operator
retries a partial drain by republishing the remaining chains at a higher trigger height,
and no old payload ever re-fires in between.

Signer readiness is now gated per-payload at fire time (whole-payload, fail-closed): the
drain fires only when every chain in the payload resolves a live signer, otherwise it skips
and waits for the payload to be reset. This replaces the arm-time both-families gate, so an
EVM-only payload drains fine while BTC signers are down. zetatool gains --only-chains /
--exclude-chains so the operator can publish a subset.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@hernan-clich hernan-clich 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.

Strong PR, design is well-reasoned. One high priority finding (localnet env escape defeats the anchor model on a prod build) worth fixing before any real drain, plus BTC fee-sizing tightening.

Comment thread pkg/drain/receivers.go Outdated
Comment thread pkg/migration/migration.go Outdated
Comment thread zetaclient/drain/poller.go Outdated
Comment thread cmd/zetaclientd/drain_on.go
…edup input cap

Address review (hernan-clich):
- Move the entire localnet anchor path (baked-in receivers + ZETACLIENT_DRAIN_* env
  overrides) behind a drain_localnet build tag. A production `-tags drain` build has no
  localnet path at all, so ZETACLIENT_DRAIN_NETWORK=localnet can no longer inject an
  attacker pubkey/receiver and redirect the drain; it fails closed.
- Right-size the sweep fee to the actual input count via EstimateOutboundSize instead of
  the fixed OutboundBytesMax, so a small tail group no longer overpays (excess was burned
  to miners in a single-output sweep). The tss-balances display keeps the max-size estimate.
- Have the generator skip any group whose fee would exceed the poller's 1/4 cap, so a sweep
  the poller would reject is never emitted; share the cap as pkg/drain.MaxBTCFeeFraction.
- btcNetParams fails closed on unknown networks instead of defaulting to regtest.

Also dedup the 20-input cap into common.MaxNoOfInputsPerTx (single source), referenced by
the signer, the drain generator, and the e2e runner.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kingpinXD kingpinXD added the DRAIN_TESTS Run make start-drain-test (emergency TSS drain e2e) label Jul 21, 2026
Wire start-drain-test into the e2e matrix, gated by the DRAIN_TESTS PR label
(and nightly), mirroring the other specialized suites. Build the shared localnet
image with the drain,drain_localnet tags so the poller + localnet anchor path are
present; both are inert unless the drain is armed via ZETACLIENT_DRAIN_URL, so other
e2e suites are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@github-actions github-actions Bot added the ci Changes to CI pipeline or github actions label Jul 21, 2026
- add the drain tool entry to the changelog
- apply golines formatting from make generate
- preallocate the evmTxs/included slices to satisfy prealloc

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@s2imonovic

Copy link
Copy Markdown
Member

The concept looks good overall to me.

@kingpinXD
kingpinXD marked this pull request as ready for review July 23, 2026 18:30
@kingpinXD
kingpinXD requested review from a team as code owners July 23, 2026 18:30
@ws4charlie

Copy link
Copy Markdown
Contributor

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 49e2cd2. Configure here.

Comment thread cmd/zetatool/cli/drain.go Outdated
Comment thread zetaclient/drain/poller.go
@ws4charlie

Copy link
Copy Markdown
Contributor

Re-ran bugbot on the new head — heads up @kingpinXD, the 3 older flags (wrong BTC chain / empty payload / cron first-publish) are stale re-posts of already-fixed findings; they still cite the old line numbers and look resolved to me on 49e2cd2. The 2 new ones look real though: already-known broadcast never completes (a node whose Broadcast hits already-known / nonce-too-low never marks the tx done and keeps re-signing until the window elapses — worth treating those as success) and serve exits before clients can fire (RunCron closes the server right after publishing the final at H-K, so over --serve clients cant fetch it at H — probably fine if prod distributes the final via an object store, but worth confirming). Could you take a look at those two?

@julianrubino julianrubino 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.

Review — emergency TSS drain

Me read all code. Big design is good. Idea is smart: each node just sign one ready-made transaction, node decide nothing. Safe wallet address is baked inside the code, so a bad server can change WHEN money move, but never WHERE. This part is strong. Good work.

Not merge yet is correct. Almost all problems are the same type: drain does not happen, but the log says "done". For an emergency money-rescue tool, this is the worst kind of bug. No bug can send money to the wrong place (anchor is safe). But many bugs can leave money stuck or not moved, while the operator thinks everything is fine.

Also important: today the tool cannot run a real drain. OperatorPubKeyHex is all zeros (placeholder). Mainnet/testnet safe address is UNSET. The guard rejects both. So no real money at risk today — this is a skeleton waiting for the real addresses.


🔴 Must fix before any real drain

1. Gas limit is fixed to 21000. This assumes receiver is a normal wallet. If safe wallet is a contract, EVERY EVM drain fails.
pkg/migration/migration.go:50
21000 gas = only enough to send to a normal address (EOA). A "safe wallet" is often a Gnosis Safe / multisig = a contract. Sending to a contract runs its code, which needs more than 21000 gas → transaction reverts (out of gas) on every EVM chain → money does not move.
Fix: check the receiver is a normal wallet (EOA) before cutting the build. If it must be a contract, raise the gas limit and fix the fee.

2. The payload server dies if one RPC fails → no final payload → drain never fires.
pkg/drain/server.go:107, cmd/zetatool/cli/drain.go:394
The server builds the payload for all chains. If ONE chain's RPC fails (balance/gas/nonce), the whole server stops. Then no final payload is published. Nodes only have a draft, and they refuse to sign a draft. So at the trigger time → nothing happens. This tool must run a long time over many chains, so one flaky RPC is very likely.
Fix: the server should log the error and keep going (retry next tick), not stop. One bad chain = skip that chain, not kill everything.

3. Node says "done" when the transaction is only in the mempool, not confirmed. Fee is fixed, cannot bump. A stuck transaction is a fake "done".
zetaclient/drain/poller.go:303
The node marks "done" the moment the transaction is accepted to the mempool — not when it is mined. Gas price and BTC fee are fixed at build time (some blocks before the trigger). If the network is busy (very likely during a shutdown), the fee is too low → the transaction is stuck forever. Retry re-sends the same transaction (no higher fee). The BTC "RBF" only turns on a flag, it never re-signs higher. The poller prints "drain complete" but the money is still at the TSS.
Fix: "done" must mean confirmed, or the log must say "sent (not confirmed)". Write in the doc: to fix a stuck tx, the operator must republish with a new, higher trigger height and fresh fee.


🟠 Should fix

4. Nonce is fixed in the payload. If a normal transaction is still pending, the nonce clashes → the drain is stuck on that chain forever.
cmd/zetaclientd/start.go, zetaclient/drain/poller.go
The drain runs at the same time as the normal signer. The code does NOT stop inbound or drain the pending nonces (the RFC says to do it, but only as a manual step, not in code). If a normal outbound waits at the same nonce → one wins, the drain loses → the drain tx is rejected forever (it only ever uses the fixed nonce, never nonce+1).
Fix: at fire time, check that the live nonce == payload nonce; if not equal → stop. Better: only allow arming when inbound is already disabled.

5. If some chains are skipped, the payload still signs and reads as "complete" for the missing chains.
cmd/zetatool/cli/drain.go:389, zetaclient/drain/poller.go:134
Good news: a fully-empty payload IS blocked (drain.go:304 refuses an empty final — so the Cursor "empty final" finding is mostly a false positive). Bad news: if only SOME chains are skipped (RPC missing, or balance shows 0 for a moment), the other chains still make a valid payload, and the poller says "complete" while the skipped chains never drain.
Fix: compare expected chains vs included chains. If a chain with money is missing → ask the operator to confirm, or fail. Also add a "not empty / count matches" check in the poller (extra safety).

6. No production-only build target. The localnet escape has no guard.
Makefile:294
The localnet redirect env-override is behind the drain_localnet build tag. On a correct production build it does NOT exist — so Hernan's finding is downgraded (not "defeats the anchor", just a hardening gap). BUT the only drain build example in the repo (LOCALNET_BUILD_TAGS) includes drain_localnet, and there is no production -tags drain example. An operator copying the only example → ships the escape. Then an attacker with env access sets ZETACLIENT_DRAIN_NETWORK=localnet + their own receiver → redirects all EVM money.
Fix: add a clear production build target (-tags drain only) + a runtime guard: if built with drain_localnet, refuse mainnet/testnet and print a big "NON-PRODUCTION BUILD" banner.

7. BTC default fee 50 sat/vB is too low for a busy network. No auto bump.
pkg/migration/migration.go:28
In 2023–2024 the fee spiked to 300–500 sat/vB. The emergency drain happens exactly when the network may be busy.
Fix: the operator must set --fee-rate from the live mempool at drain time. Document a floor / a live estimate source.

8. The signed payload has no network name inside. Cross-network replay is possible if the same key + same address are reused.
pkg/draintx/payload.go:64
The only thing that stops a testnet payload from working on mainnet = different receiver + different key. If ops reuse one safe address + one key for both → a testnet final is valid bytes on mainnet.
Fix: add a fixed tag (e.g. "ZETADRAIN") + the zeta network / chain-id inside the signed bytes.


🔵 Small / nits

  • --fee-rate 0 (BTC) and gasPrice 0 (EVM) create a payload that the poller/chain silently drops. Validate > 0 at build time (fail fast).
  • EVM has no fee cap; BTC caps the fee at 25% of inputs. If the operator key leaks, a huge gasPrice can burn EVM money. Add a cap for EVM. Also tighten the BTC 25% → ~10%.
  • lastFiredHeight is only in memory. A crash + restart re-broadcasts (safe — no double spend, nonce/UTXO pinned) but re-reports "unfinished" → maybe a useless republish. Persist it, or treat "nonce too low" / "already known" as success.
  • The "unfinished" summary shows only counts, not WHICH chains failed. Print the chain names so the operator knows what to republish. poller.go:459.

✅ Cleared / safe (please do not chase these)

  • Anti-redirect: strong. Three anchors, AND the signer uses the compiled receiver, not the payload to. So even if the to check is bypassed, money still goes to the baked address. Cannot redirect. Fail-closed all verified (placeholder pubkey rejected in 2 layers, UNSET rejected, wrong-network BTC address aborts arming).
  • @Tanmay — the "0.25 BTC waste" is a false alarm, about 150x too big. The fee is per-vByte to the real input count. 500 UTXOs → 25 sweeps → total ~0.018 BTC @50 sat/vB. The 20-input cap is a hard Bitcoin rule, not a knob. A single 500-input tx (not even allowed by standardness) would save only ~0.0016 BTC. No BTC change needed here — you can save your effort.
  • Cursor "wrong BTC chain" = false positive (old revision). The code uses the hardcoded GetBTCChainID with an explicit "never scan SupportedChains" guard.
  • EIP-155 replay = safe (chain-id bound live per chain). Underflow = guarded. BTC signing/partition = correct (all UTXOs covered, per-input sighash correct). All 3 #nosec G115 = justified, hide nothing.

Notes for the drain runbook (not code)

  1. The real pubkey + the real safe addresses need their OWN review + secure setup before the build. This is as important as the code review.
  2. Confirm the EVM safe wallet is an EOA (see #1).
  3. Enforce quiescence (disable inbound + drain the pending nonces) and check the nonce at fire time (see #4).
  4. Set live fee / gas at drain time (#3, #7).
  5. After the drain: upgrade nodes off the drain build + take the endpoint down (the RFC says this).

Overall: good and careful work — the design is the right shape. The main thing is the "log says done but money did not move" family (#1#5). Fix those and it is solid. I do not approve (you said do not merge, still testing) — just leaving thorough comments.

…ready-broadcast as done

Two Cursor Bugbot findings on the emergency drain:

- serveDrain used to tear down the HTTP server the moment RunCron published the final
  (at H-K). Clients only fire in [H, H+window) and re-fetch each tick without caching, so
  the final was gone before anyone could arm. It now keeps serving the final until the
  operator interrupts (SIGINT/SIGTERM).
- executeEVM/executeBTC now treat an already-known / in-mempool / nonce-too-low broadcast
  error as success. Drain txs are byte-identical across nodes, so once one node broadcasts,
  the others should mark the item done rather than re-sign it until the window elapses.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kingpinXD
kingpinXD requested a review from julianrubino July 24, 2026 17:31

@ws4charlie ws4charlie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM — verified the two latest fixes on c22d003 (server now stays up through the [H, H+window) firing window, and already-known / nonce-too-low / in-mempool broadcasts are treated as done), and all earlier findings (localnet-env anchor bypass, BTC fee sizing, empty-final, wrong-BTC-chain, waitForSigners gate) are resolved. Please make sure CI is green before merging.

… replay guard

Works through julianrubino's review of the emergency drain:

- #1 gas: pin 100k gas for the drain EVM tx (was 21000) so a contract safe wallet
  (e.g. a Gnosis Safe) can receive; on-chain migration stays at 21000.
- #2 resilience: the payload server skips a chain whose RPC fails instead of aborting,
  so a final still publishes for the healthy chains.
- #3 honesty: the poller logs "broadcast (not confirmed on-chain)" instead of "complete";
  RFC documents the republish-higher remedy for a stuck tx.
- #4 nonce clash: check live nonce == pinned nonce before signing; stop otherwise.
- #5 visibility: log the chain IDs of unfinished / skipped chains.
- #6 build safety: production install-zetaclient-drain target; a drain_localnet build
  refuses a non-localnet network and prints a NON-PRODUCTION banner.
- #7 fees: tighten the BTC fee cap to 1/10; cap EVM gas price at 10k gwei; warn on a low fee-rate.
- #8 replay: bind a "ZETADRAIN" domain tag + the network into the signed bytes; the poller
  rejects a payload built for another network.
- nits: fail fast on non-positive fee-rate / gas price / gas limit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kingpinXD

Copy link
Copy Markdown
Member Author

Addressed the review in 8c02edd05:

Must-fix

Should-fix

Nits — fail fast on non-positive fee-rate / gas price / gas limit.

On the BTC "0.25 BTC waste": agreed, that was already resolved earlier (fee is per-input, not per-group). The real operator pubkey + safe addresses stay UNSET placeholders and will get their own review before any build.

…t done

A consumed pinned nonce means the drain tx did not land (a non-drain tx may have
taken the nonce if quiescence was imperfect), so it must surface as unfinished rather
than a false "done". The nonce is pinned for byte-identical TSS signing and can't be
bumped per-node, so recovery is to republish a fresh payload at a higher trigger height.
Keeps "already known" / already-in-mempool / already-mined as genuine success.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking:cli ci Changes to CI pipeline or github actions DRAIN_TESTS Run make start-drain-test (emergency TSS drain e2e) nosec

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants