Atomic swaps: fix offer publishing, harden fee/balance validation, generic Ethereum RPC endpoint, any-ERC-20 and Confidential-Asset swaps#2075
Open
Maxnflaxl wants to merge 8 commits into
Open
Conversation
Since d4d0e78 ("AtomicSwapTransaction - using nonce addresses") FillSwapTxParams() allocates a bare key id and never saves a WalletAddress. SwapOffersBoard only recognizes publishers found in IWalletDB::getAddresses(true), so every newly created offer was rejected with ForeignOfferException ("Offer has foreign Pk and will not be published") — swap offer publishing has been broken since 7.3. Create and persist the publisher address in FillSwapTxParams(), as the v6 API's createWID() did before the change. The address is saved with Auto expiration so it ages out of the address book; it must outlive the offer so SwapOffersBoard can broadcast terminal status updates and keep classifying the offer as own. Fixes BeamMW/beam-ui#1017, BeamMW/beam-ui#1073.
InitSwap and AcceptSwap never compared the swap amount against the side-chain balance, so a swap could be created that exceeds the ether/usdt/wbtc/dai (or BTC-family) balance and only fail later, deep in the lock-tx phase. Reuse the synchronous GetBalance() helper and reject the swap up front when the sender's swap-coin balance is insufficient.
The pre-flight validator assumes a fixed-size lock tx, so with many small UTXOs the funded transaction can be far larger than estimated and the configured fee rate insufficient — the swap then stalls with an unconfirmable lock tx. Surface the actual fee from fundRawTransaction (bitcoin-core returns it; Electrum computes it during coin selection) and fail the LOCK_TX subtransaction with FeeIsTooSmall before sign/broadcast when fee/vsize is below the configured rate, telling the user to consolidate coins or raise the fee rate.
The bridge hardcoded Infura's URL shape (host, /v3/<projectID> path, constant SSL). Add a custom-RPC mode with a strictly validated endpoint URL (http/https only, no credentials, IPv4/IPv6/hostname + port + path) so Alchemy, QuickNode, Chainstack, Ankr or a self-hosted node all work; SSL now derives from the URL scheme. New getChainID bridge RPC and Client::ValidateEndpoint() report chain id + latest block for connection feedback before a swap is attempted. Existing settings load unchanged as Infura mode. Endpoint URLs never reach the logs unsanitized (the path can contain the provider API key).
ERC-20 half: AtomicSwapCoin::Erc20Token with per-offer contract address, symbol and decimals in tx parameters 41-45 (frozen by static_asserts). Extended offers broadcast with the ExtendedOffer wire coin so pre-change wallets drop them via their existing m_coin >= Unknown guard; classic offers stay byte-identical (golden-vector test). Token metadata is read from the contract (symbol()/decimals() via eth_call) and always confirmed explicitly; decimals bounded by kMaxTokenDecimals = 18. CA half: a Confidential Asset can take the Beam side. The shared HTLC output carries the asset: the commitment is built on the asset generator, the cosigned rangeproof runs over its blinded form (CoSign with pHGen), and the surjection proof is built by both parties from a generator-blinding scalar derived from the shared bulletproof seed, so neither trusts the other about the asset id. Refund/redeem withdraw the full asset amount; the kernel fee stays BEAM and is funded from the tx owner's own coins - the asset redeemer must hold BEAM for the redeem fee (validated and surfaced in CLI/API). swap_test resurrected as the e2e vehicle with CA lock/redeem and refund scenarios in both directions. Follow-ups from review: eth-address validation consolidated into IsValidEthContractAddress, used by the CLI, the API and the offers board. The API's Erc20Token pre-flight balance check stays a documented no-op - its handlers are synchronous with no live-balance-query path, unlike the CLI's live eth_call - now gated on contract-address format with an accurate comment instead of a stale one; an insufficient balance still fails when the lock tx is built. Part of #1166; see #2070.
The fetched-kernel path of ProofKernel2 handling threw ThrowUnexpected when the returned kernel IS valid, killing the node connection on every successful fetch. The pre-refactor code and the sibling Kernel3 branch both throw on !IsValid. Exercised by atomic-swap redeem-secret extraction, where the inverted check turned every redeem into an endless reconnect loop.
Mainnet gas has been well below 15 gwei since the merge; the floor made EstimateGasPrice discard every real node suggestion (recommended rate 0, shown as a connection error) and rejected any manually entered fee below 15 gwei. Keep a 1 gwei floor and clamp the recommendation up to it instead of zeroing it out.
…ry into helpers Retires the pre-existing TODO next to the conditions-accept loop.
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.
Atomic swaps have been broken since 7.3 and accumulated a set of open issues. This PR fixes the breakage, hardens the validation around swaps, and extends them to arbitrary ERC-20 tokens and Beam Confidential Assets. One commit per issue plus two small standalone commits (a stale fee-rate constant, CLI cleanups); tracking issue #2070.
Fixes
#2071 — swap offers rejected as foreign. Since d4d0e78 ("using nonce addresses")
FillSwapTxParams()allocated a bare key id and never persisted aWalletAddress, butSwapOffersBoardonly recognizes publishers found ingetAddresses(true)— every offer threwForeignOfferException, breaking publishing in UI, CLI and API (BeamMW/beam-ui#1017, BeamMW/beam-ui#1073). The publisher address is now created and saved withAutoexpiration; a board regression test drives the realFillSwapTxParams → publishOfferpath.#1697 — CLI allowed swaps exceeding the swap-coin balance.
swap_initand the accept path now check the live side-chain balance (ether/tokens/BTC family) before starting.#1039 — lock-tx fee rate unverified after funding. The pre-flight validator assumes a fixed-size tx, so many small UTXOs could produce an unconfirmable lock tx.
fundRawTransactionnow surfaces the actual fee (bitcoin-core returns it; Electrum computes it during coin selection) andBitcoinSidefails the LOCK_TX subtransaction withFeeIsTooSmallwhenfee/vsizeis below the configured rate. The acceptance bound tops each unsigned input up from its 41-byte serialized skeleton to the 68-vbyte signed P2WPKH lower bound, so segwit-funded transactions cannot false-positive.Stale 15 gwei mainnet fee-rate floor. Mainnet gas has been well below 15 gwei since the merge, so the floor made
EstimateGasPricediscard every real node suggestion (recommended rate 0, surfaced as a connection error) and rejected any manually entered fee below 15 gwei. The floor is now 1 gwei and the recommendation is clamped up to it instead of zeroed.#2073 — inverted kernel-validity check in
fly_client.cpp. Found while resurrectingswap_test: the fetched-kernel path ofProofKernel2handling threwThrowUnexpectedwhen the kernel IS valid (refactor regression from 7a38b48), turning every swap redeem into an endless reconnect loop. One-character fix restoring the pre-refactor semantics; without it the first full-swap test scenario hangs, with it the suite passes.Features
#2072 — generic Ethereum JSON-RPC endpoint. The bridge hardcoded Infura's URL shape. Settings gain a custom-RPC mode with a strictly validated endpoint URL (http/https only, hostname or IPv4, port 1–65535, no embedded credentials; bracketed IPv6 literals are rejected because the HTTP transport is IPv4-only and would silently never connect; endpoint URLs never reach logs unsanitized since provider paths carry API keys). New
getChainIDRPC plusClient::ValidateEndpoint()report chain id and latest block before a swap is attempted; mainnet builds reject non-mainnet chains. Existing stored settings load unchanged as Infura mode.BeamMW/beam-ui#1166 — any ERC-20 token and any Confidential Asset. - Wire format: new
AtomicSwapCoin::Erc20Tokenand anExtendedOffermarker appended at the oldUnknownordinal, plus tx parameters 41–45 (beam asset id/name, token contract/symbol/decimals), all frozen by static_asserts. Extended offers broadcast with theExtendedOfferwire coin so pre-change wallets drop them via their existingm_coin >= Unknownguard; classic offers stay byte-identical (locked by a golden-vector test). - ERC-20 side: per-offer token contract;symbol()/decimals()read viaeth_call(ABI-string decode with bytes32 fallback, exposed throughClient::GetTokenInfo) and always shown for explicit confirmation; decimals bounded (kMaxTokenDecimals = 18) at the board, the bridge decode and the unit helpers to prevent amount mis-scaling; contract and token parameters validated by one shared helper (IsValidEthContractAddress/GetValidatedErc20Params) in the CLI, the API and the offers board; CLIswap_coin=erc20 --token_contractand API mirror. TheErc20Tokensecond side registers the Ethereum factory, and the board's incomplete-offer recovery re-broadcasts a terminal status with the full parameter set (a parameterless extended-offer update would be dropped by every peer). - Confidential-Asset side: an asset can take the Beam side of the HTLC. The shared output's commitment is built on the asset generator; the cosigned rangeproof runs over its blinded form (CoSignwithpHGen); the surjection proof is derived by both parties from a generator-blinding scalar hashed from the shared bulletproof seed, so neither party trusts the other about the asset id (this mirrors core's ownOutput::CreateMPC construction — no consensus changes). Refund/redeem withdraw the full asset amount; the kernel fee stays BEAM, funded from the tx owner's own coins, so the asset redeemer must hold BEAM for the redeem fee (validated and surfaced in CLI/API). - Plumbing fixes surfaced by asset withdraws:AddSharedInputno longer drops the shared input when owner-side inputs exist, and completed-swap coin statuses are applied per sub-transaction (with the unused refund/redeem reservation released) instead of txID-wide.Testing
swap_testresurrected as the e2e vehicle (disabled since 2023;wallet/unittests/CMakeLists.txt). Full suite green in ~3.5 min: classic swaps (4 directions), CA swaps in both directions against a real mining node (the shared asset output passed actual consensus validation), CA refund, BTC/Beam refunds, cancel/expire matrix, electrum variants. Two scenarios (TestSwapTransactionWithoutChange,TestIgnoringThirdPeer) remain disabled: pre-fork3 a kernel proof verifies only while its block is the tip, and the instant-advancingTestNodealways loses that race — a test-environment limitation with no mainnet relevance (mainnet is past fork3).swap_board_testextended: publisher-address regression, raw-wire-coin rejection, decimals bounds, wire-stability golden vector, extended-offer round trip with on-wire snooping.eth_rpc_endpoint_test(URL parser accept/reject matrix, settings round-trip, decimals-word parsing, token-param tokenization round trip);bitcoin_rpc_testextended for the funded-fee bound;wallet_api_testcovers the token pre-flight balance decision helper.Notes for review
issue/1267) depends on this branch's head via its submodule pointer — merge/push this PR first.Closes #2071, #1697, #1039, #2072, #2073. Core half of BeamMW/beam-ui#1166. Part of #2070.