bitcoin-wallet: Consider unconfirmed parent fees during PSBT fee estimation#977
bitcoin-wallet: Consider unconfirmed parent fees during PSBT fee estimation#977xn1xn1 wants to merge 4 commits intoeigenwallet:masterfrom
Conversation
|
thank you for the contribution! please disclose the extend to which this was LLM generated. |
xn1xn1
force-pushed
the
master
branch
3 times, most recently
from
83e19fd to
f0d5114
Compare
Hi there. I did use Claude (and gpt5) with adaptive thinking to generate some barebones pseudocode which I used to work on further, as I'm not familiar with this code base. I rechecked and reviewed the output multiple times though. Let me know if there are any issues. If this repo is strictly no AI, then feel free to close this pull request. |
|
Can squash once you review the new commit. |
|
Have you tested this manually ? |
I did before making some minor change, looks like I misplaced a semicolon. I did now, sorry. 😓 |
|
@Einliterflasche Review would be good here. |
|
bugbot run |
|
We need a testcase for this. In the regtest environment, broadcast a tx with low fee and then check if the child tx has increased the fee accordingly. |
And where should that testcase be? |
|
The bitcoin test harness is in |
|
@Einliterflasche Can you have a look at the test? |
| fn psbt_fee_sats(psbt: &bitcoin::psbt::Psbt) -> u64 { | ||
| let input_value: u64 = psbt | ||
| .inputs | ||
| .iter() | ||
| .map(|input| { | ||
| input | ||
| .witness_utxo | ||
| .as_ref() | ||
| .expect("missing witness_utxo") | ||
| .value | ||
| .to_sat() | ||
| }) | ||
| .sum(); | ||
|
|
||
| let output_value: u64 = psbt | ||
| .unsigned_tx | ||
| .output | ||
| .iter() | ||
| .map(|output| output.value.to_sat()) | ||
| .sum(); | ||
|
|
||
| input_value.saturating_sub(output_value) | ||
| } |
There was a problem hiding this comment.
there is a function Psbt::fee which does the same
|
|
||
| let parent_tx = wallet.sign_and_finalize(parent_psbt).await?; | ||
| let parent_txid = parent_tx.compute_txid(); | ||
|
|
There was a problem hiding this comment.
add an assert_eq! here to make sure this tx actually has the fee we think it has (parent_fee).
| Ok(()) | ||
| }) | ||
| .await; | ||
|
|
There was a problem hiding this comment.
This test requires that:
- the child fee makes the fee rate of the package at least 1sat/vB
- the child increases the package fee rate
We should also require that:
- the child fee rate is greater than the parent fee rate
and test this with 2 level of transaction depth (child pays for parent and grand parent) and also test that the negative cases. Like testing that we only pay for the parent and grand parent but not great-grand parent etc. and also testing that a sanity limit on the fee is still enforced.
3 participants
Closes #456.