fix: avoid lazy RegTestParams construction in ZeroConfCoinSelector#298
fix: avoid lazy RegTestParams construction in ZeroConfCoinSelector#298HashEngineering wants to merge 1 commit into
Conversation
…ector isTransactionSelectable() compared a pending tx's params against RegTestParams.get() for every pending output it evaluated. RegTestParams.get() lazily runs the RegTestParams constructor, which recomputes the regtest genesis X11 hash and checkState()s it against a hardcoded value. When that lazy construction is first triggered on a contended background thread (e.g. CoinJoin's IO coroutines doing heavy concurrent X11 hashing) a transient bad X11 result makes the checkState throw, crashing coin selection / balance calculation - even on mainnet/testnet wallets that never use regtest. Compare against NetworkParameters.ID_REGTEST instead (a plain field lookup, no construction or genesis recompute). Behavior is unchanged: RegTestParams sets id = ID_REGTEST, so the regtest special-case still applies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough
ChangesNetwork ID comparison for regtest eligibility
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Problem
ZeroConfCoinSelector.isTransactionSelectable()compares a pending transaction's params againstRegTestParams.get()for every pending output it evaluates:RegTestParams.get()lazily runs theRegTestParamsconstructor, which recomputes the regtest genesis X11 hash andcheckState()s it against a hardcoded constant.When that lazy construction is first triggered on a contended background thread — notably the wallet's CoinJoin IO coroutines, which do heavy concurrent X11 hashing — a transient bad X11 result makes the
checkStatethrowIllegalStateException, crashing coin selection / balance calculation. This happens on mainnet/testnet wallets that never use regtest.Observed crash (dash-wallet, CoinJoin balance update):
The hardcoded regtest genesis constant is correct — verified that
RegTestParams.get()succeeds under a correct X11 — so the failures are transient corruption of the X11 computation, not a stale constant.Fix
Compare against
NetworkParameters.ID_REGTEST(a plain field lookup) instead of constructingRegTestParams. Behavior is unchanged:RegTestParamssetsid = ID_REGTEST, so the regtest special-case still applies — it just no longer recomputes/asserts the genesis hash on a hot, concurrent path.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes