feat(go): add testnet4 mapping and harden runtime integration behavior#4002
Open
lionakhnazarov wants to merge 9 commits into
Open
feat(go): add testnet4 mapping and harden runtime integration behavior#4002lionakhnazarov wants to merge 9 commits into
lionakhnazarov wants to merge 9 commits into
Conversation
…lows Separates hardhat/deploy/runtime Solidity logic from CI and generated deployment artifacts to make contract behavior review focused and tractable. Co-authored-by: Cursor <cursoragent@cursor.com>
Moves generated deployment snapshots and ancillary artifact churn into a standalone layer so functional Solidity and Go changes can be reviewed independently. Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced May 25, 2026
…f runtime paths Introduces testnet4 configuration and electrum URL wiring, updates chain integrations including DKG validator support, and improves btcdiff/TBTC runtime and reliability-oriented tests. Co-authored-by: Cursor <cursoragent@cursor.com>
lionakhnazarov
force-pushed
the
stack/testnet4-03-deployment-artifacts
branch
from
72b052e to
5b40e26
Compare
lionakhnazarov
force-pushed
the
stack/testnet4-04-go-testnet4-runtime
branch
from
99015a1 to
dc687c9
Compare
WaitMined previously used context.Background() with no timeout, and the follow-up confirmation-depth wait called the context-less BlockCounter.WaitForBlockHeight. If the RPC stalls or the chain stops producing blocks, the maintainer would hang indefinitely on every Retarget / RetargetWithRefund call. Wrap both waits under a 10-minute shared deadline and add a context shim around WaitForBlockHeight so callers can enforce timeouts on the context-less BlockCounter interface.
Elevate the Sepolia/Developer defaultGroupParameters log to Warn level and spell out that GroupQuorum equals GroupSize (3/3/2), so all three operators must stay online for DKG to progress. Also emit a Warn when the EcdsaDkgValidator contract address is not configured. The fallthrough was previously silent, leaving operators unaware that group sizing was coming from compile-time defaults rather than the on-chain validator.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
piotr-roslaniec
force-pushed
the
stack/testnet4-04-go-testnet4-runtime
branch
from
754346d to
dc687c9
Compare
piotr-roslaniec
force-pushed
the
stack/testnet4-03-deployment-artifacts
branch
from
9ade31f to
5b40e26
Compare
5 tasks
## Stack Context Follow-up to #4002 (`stack/testnet4-04-go-testnet4-runtime`). Base: `stack/testnet4-04-go-testnet4-runtime` Addresses three runtime/operability issues surfaced during review of #4002. ## What's in this PR ### 1) Bound the synchronous retarget wait — `pkg/chain/ethereum/bitcoin_difficulty.go` `waitDeployBackendTransactionMined` previously called `bind.WaitMined(context.Background(), …)` with no timeout, then `BlockCounter.WaitForBlockHeight` (which has no context parameter). A stalled RPC or chain that stopped producing blocks would hang the maintainer indefinitely on every `Retarget` / `RetargetWithRefund` call — including on mainnet. Both waits are now bounded under a shared 10-minute deadline. A small `waitForBlockHeightCtx` shim adapts the context-less `BlockCounter` interface, with a regression test verifying it returns `context.DeadlineExceeded` when the counter blocks forever. ### 2) Warn loudly about Sepolia DKG fragility — `pkg/tbtc/tbtc.go`, `pkg/chain/ethereum/tbtc.go` The Sepolia/Developer `defaultGroupParameters` returns `{GroupSize:3, GroupQuorum:3, HonestThreshold:2}`. Quorum equals size — a single offline operator prevents DKG progress. - Elevated the existing `Infof` to `Warnf` and spelled out the operational consequence. - Added a `Warnf` in `pkg/chain/ethereum/tbtc.go` where the `EcdsaDkgValidator` `ErrAddressNotConfigured` branch previously fell through silently. Operators now see at startup whether group sizing is coming from on-chain values or compile-time defaults. No sizing values are changed. ### 3) Document the testnet3 → testnet4 wiring as breaking No code change in this PR; the startup banner at `config/config.go:101` already prints the resolved Bitcoin network. Operator-facing note is captured here and should be folded into release notes when #4002 lands. After #4002 lands, `network.Type=Testnet` (Sepolia) resolves to `bitcoin.Testnet4` rather than `bitcoin.Testnet` (testnet3). Existing Sepolia operators will switch both the embedded Electrum URL set and the Bitcoin network on upgrade. Pin to a pre-#4002 build to remain on testnet3. ## Test Plan - [x] `go build ./...` - [x] `go vet ./pkg/chain/ethereum/... ./pkg/tbtc/...` - [x] `go test ./pkg/chain/ethereum/...` - [x] `go test ./pkg/tbtc/` - [x] New regression: `TestWaitForBlockHeightCtx_DeadlineExceeded`, `TestWaitForBlockHeightCtx_ReturnsImmediatelyOnSuccess`
lionakhnazarov
force-pushed
the
stack/testnet4-03-deployment-artifacts
branch
from
3b156d8 to
979333c
Compare
When the Electrum fee oracle returns no estimate for any confirmation target, EstimateSatPerVByteFee returned a hardcoded 2 sat/vByte for every network. On mainnet this turns a fail-safe (error, no broadcast) into a fail-broadcast: the tBTC sweep and redemption fee paths build and broadcast transactions at a feerate that can be left unconfirmable or evicted under congestion, stalling sweeps and redemptions. Gate the fallback to test networks (testnet, testnet4, regtest) via a new Electrum Config.Network field resolved from the client configuration. On mainnet, and on any unset or unknown network, the oracle failure is surfaced as an error so callers do not broadcast at a guessed feerate. The field is tagged mapstructure:"-" and set after config unmarshal, so a config key cannot re-enable the fallback on mainnet. Add a unit test covering the fallback decision per network and propagate the resolved network into the Electrum integration test configs.
…#4019) ## Problem PR #4002 added an Electrum fee-estimate fallback: when `blockchain.estimatefee` returns no estimate for any confirmation target (common on quiet testnet4 mempools, which answer `-32603` for every target), `EstimateSatPerVByteFee` returns a hardcoded `2` sat/vByte instead of an error. That fallback is network-blind. Before, an oracle failure returned an error and no transaction was broadcast on a guessed feerate (fail-safe). After, the same failure yields a fixed `2` sat/vByte that flows into the production tBTC proposal fee paths (`tbtcpg/deposit_sweep.go`, `redemptions.go`, `moving_funds.go`, `moved_funds_sweep.go`) on mainnet as well, with no minimum-feerate floor in `TransactionFeeEstimator.EstimateFee`. On mainnet a `2` sat/vByte transaction can be left unconfirmable under congestion, or evicted if the dynamic mempool minimum feerate rises above it — stalling sweeps and redemptions (delayed mints / undelivered BTC). The funds are not lost, but operations can get stuck. ## Solution Gate the fallback to networks where an underpriced transaction is economically harmless — `testnet`, `testnet4`, `regtest`. On `mainnet`, and on any unset or unknown network, the oracle failure is surfaced as an error so callers do not broadcast at a guessed feerate (fail-closed default). - A new `electrum.Config.Network` field carries the resolved Bitcoin network into the connection. It is set from the client configuration in `config.resolveElectrum`, tagged `mapstructure:"-"`, and assigned after config unmarshal — so a config-file key cannot re-enable the fallback on mainnet. - The fallback decision is factored into pure helpers (`feeFallbackResult` and `lowFeeFallbackAllowed`) so it can be tested deterministically without a live Electrum client. - testnet4's existing graceful degradation is preserved; only mainnet (and unknown-network) behavior changes — back to the pre-#4002 fail-safe. ## Tests - New unit test `TestFeeFallbackResult` covers the decision per network: mainnet and unknown fail safe with an error; testnet/testnet4/regtest use the fallback; transport-level failures always error. - `config.TestResolveElectrum` now asserts the resolved network is propagated into the Electrum config. - The Electrum integration test configs carry their network so the integration path exercises the gate rather than defaulting to `Unknown`. - Local CI is green: `gofmt -l`, `go vet`, `go build ./...`, `go test` (changed packages), `go vet -tags=integration` (compile), Staticcheck 2025.1.1, and gosec (0 issues). ## Notes This is a follow-up to the review of #4002 and is based on its head branch (`stack/testnet4-04-go-testnet4-runtime`), so the diff is scoped to this fix.
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
3 participants
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.
Stack Context
This is PR 4/4 in the reorganization stack.
Base: #4001 (
stack/testnet4-03-deployment-artifacts)What Changed
1) CLI/config wiring for Testnet4
cmd/flags.go,cmd/flags_test.go,cmd/start.goconfig/_electrum_urls/testnet4config/network/network.goconfig/config_test.goconfig/electrum_test.goconfig/contracts.go2) Bitcoin/Electrum behavior and tests
pkg/bitcoin/bitcoin.gopkg/bitcoin/electrum/electrum.gopkg/bitcoin/electrum/electrum_integration_test.gopkg/bitcoin/electrum/electrum_test.go3) Ethereum chain integration + TBTC wiring
pkg/chain/ethereum/bitcoin_difficulty.gopkg/chain/ethereum/ecdsa_dkg_validator_chain.go(new)pkg/chain/ethereum/ethereum.gopkg/chain/ethereum/ethereum_integration_test.gopkg/chain/ethereum/tbtc.go4) Runtime reliability and maintainer logic
pkg/chain/local_v1/blockcounter.gopkg/internal/dkgtest/dkgtest.gopkg/maintainer/btcdiff/bitcoin_difficulty.gopkg/maintainer/btcdiff/bitcoin_difficulty_test.gopkg/maintainer/btcdiff/config.gopkg/maintainer/btcdiff/errors.go(new)5) TBTC logic/tests and integration test config updates
pkg/tbtc/inactivity.gopkg/tbtc/inactivity_test.gopkg/tbtc/tbtc.gopkg/tbtc/coordination_test.gopkg/tbtcpg/internal/test/marshaling.gotest/config.json,test/config.toml,test/config.yamlTest Plan
go test ./config/... ./pkg/bitcoin/...go test ./pkg/maintainer/btcdiff/... ./pkg/tbtc/...go test ./pkg/chain/ethereum/...