Conversation
|
👋 Thanks for assigning @tnull as a reviewer! |
Camillarhi
force-pushed
the
feat/tx-rebroadcast-fee-bumping-follow-up
branch
6 times, most recently
from
0317cd5 to
3c78568
Compare
Camillarhi
force-pushed
the
feat/tx-rebroadcast-fee-bumping-follow-up
branch
3 times, most recently
from
072ca8b to
55d0eed
Compare
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
CI is currently unhappy:
thread 'onchain_fee_bump_rbf' (27606) panicked at tests/integration_tests_rust.rs:2552:13:
assertion `left == right` failed: node_a inbound payment txid should be updated to the replacement txid
left: d883977bf0eca143bb96ae910db99b4b570adfce48bab49d1ce928f482d685dd
right: 46db4dd87d38b1ecfb4b0760a544ee7f2d1598f42376a70a520b28bb37f5166e
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Thanks for flagging. It passes consistently on my local machine, so I'm currently investigating whether this is something environment-specific. Will update once I've dug further. |
Camillarhi
force-pushed
the
feat/tx-rebroadcast-fee-bumping-follow-up
branch
11 times, most recently
from
ae26150 to
a86a73e
Compare
Use user-provided fee rate for bumping transaction and return an error if it is too low, otherwise use system estimated fee rate and retry mechanism. Also switches the RBF test to use `random_chain_source`, and removes unnecessary sleep-based waits.
Camillarhi
force-pushed
the
feat/tx-rebroadcast-fee-bumping-follow-up
branch
from
a86a73e to
33a3af5
Compare
| wait_for_tx(&electrsd.client, new_txid).await; | ||
|
|
||
| // Sleep to allow for transaction propagation | ||
| // Give the chain source time to index the unconfirmed transaction before syncing. |
There was a problem hiding this comment.
The sleep is needed here because of test-environment-specific reasons, wait_for_tx uses the Electrum API to confirm the tx is in the mempool, but when Esplora is the chain source, Esplora may not have indexed it yet. If sync_wallets() runs in that window, BDK misses the tx and bump_fee_rbf can't find it via tx_details.
In practice, the fee of a transaction will not be bumped the instant they send. By the time a user decides to bump, the wallet has gone through multiple sync cycles, and the tx is long indexed. The sleep simulates that gap.
| match &node_a_received_payment[0].kind { | ||
| PaymentKind::Onchain { txid: inbound_txid, .. } => { | ||
| assert_eq!( | ||
| *inbound_txid, second_bump_txid, | ||
| "node_a inbound payment txid should be updated to the second replacement txid" | ||
| ); | ||
| }, | ||
| _ => panic!("Unexpected payment kind"), | ||
| } |
There was a problem hiding this comment.
Due to sync timing, the assertion verifying that node a's inbound payment txid has been updated to the replacement_txid is now done after the transaction is confirmed rather than immediately after each bump. This still fully validates the behaviour. Once confirmed, we check that the txid reflects the final bumped transaction, confirming that node A's sync correctly processed the replacement.
3 participants
Some minor follow-ups after #628 landed
Use user-provided fee rate for bumping transaction and return an error if it is too low, otherwise use system estimated fee rate and retry mechanism.
Switches the RBF test to use
random_chain_source.Closes - #797