Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invalid balance for wallet with multiple coinbase transactions #1051

Closed
Tracked by #72
turkycat opened this issue Aug 1, 2023 · 2 comments · Fixed by #1090
Closed
Tracked by #72

invalid balance for wallet with multiple coinbase transactions #1051

turkycat opened this issue Aug 1, 2023 · 2 comments · Fixed by #1090
Assignees
Labels
bug Something isn't working
Milestone

Comments

@turkycat
Copy link

turkycat commented Aug 1, 2023

Describe the bug
A wallet with more than one coinbase transaction mined to addresses owned by the wallet (or even the same address) shows only 5,000,000,000 balance (50 bitcoin) no matter how many blocks are mined to addresses owned by that wallet.

To Reproduce
I'm testing using two regtest bitcoin core nodes and a Fulcrum server. I'm also using an RPC to mine blocks on regtest nodes and test various circumstances. To eliminate as many variables as possible, I've reproduced the issue using only bdk-cli and a docker exec shell connected to one bitcoin node and using bitcoin-cli. I also verify my results with a simple Rust example near the bottom.

I believe I have sufficient evidence that the issue is not with Fulcrum, as I will explain below.

First, the setup and issue:
1)

$ bdk-cli --network regtest repl --server localhost:50001 --descriptor "wsh(multi(2,[25732241/84h/1h/0h/2h]tpubDFHnu4VcNjyj8VkUiAR5edPCRWJjrP2Mbar5BiLepxU23nEXo5mZmynzPc9h5wkqZD6bTWrsbTr1N4dryVjBMfgjYJBQUEDhzBU4NqHxQVu/0/*,[7737acb8]tpubD6NzVbkrYhZ4X3vCdevm3aFUdeSAWsbR4UhhdApbXnVUnvuwtq5NYqQ7zCkNhYG4JcPdGW4Wg4ZrVSYGpdoWsn337qmkP6CFgs7pZid12Sa/0/*,[dd7a3df3/84h/1h/0h/2h]tpubDEcxXkhzStbfzt9gPEbph3dGeR9uBMkr2rStYHcqxo9LwPDgAqbMgbYDJ8334MSesVVAwxcdYm4o7XJGjyDX6qNYEbTZov2gaMdGZjYFqXv/0/*))#00f5ljet"

>> wallet get_new_address
{
  "address": "bcrt1q6g0mhnwcssfulc39as04k4xazaxt2s6eg0q6yj4uylzzcfy73n3sw8h3pw"
}

in my docker exec terminal I generatetoaddress 1 block to address bcrt1q6g0mhnwcssfulc39as04k4xazaxt2s6eg0q6yj4uylzzcfy73n3sw8h3pw.

the block generated is 1ae02086845d99eda3f6d8ca2605086bd00affe71acf941b9cc1437eb9403983.

then, generate 100 blocks to another random trash wallet created within the bitcoin core instance. this unlocks the block subsidy generated to the address we care about.

examine the interesting block:

# bitcoin-cli -regtest -rpcuser=<redacted> -rpcpassword=<redacted> -rpcwallet=<redacted> getblock 1ae02086845d99eda3f6d8ca2605086bd00affe71acf941b9cc1437eb9403983
{
  "hash": "1ae02086845d99eda3f6d8ca2605086bd00affe71acf941b9cc1437eb9403983",
  "confirmations": 101,
  "height": 1,
  "version": 536870912,
  "versionHex": "20000000",
  "merkleroot": "ae21741e8094f71d5bf8ae2db41d3e7d9c8f5d4a246913cc228934664903d0bd",
  "time": 1690855159,
  "mediantime": 1690855159,
  "nonce": 1,
  "bits": "207fffff",
  "difficulty": 4.656542373906925e-10,
  "chainwork": "0000000000000000000000000000000000000000000000000000000000000004",
  "nTx": 1,
  "previousblockhash": "0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206",
  "nextblockhash": "4adf7ec81573d1d84f66dd5cdd886336ba032224f56b468d13a0f43bef1222ec",
  "strippedsize": 225,
  "size": 261,
  "weight": 936,
  "tx": [
    "ae21741e8094f71d5bf8ae2db41d3e7d9c8f5d4a246913cc228934664903d0bd"
  ]
}
  1. back in bdk-cli repl
>> wallet sync
{}
>> wallet get_balance
{
  "satoshi": {
    "confirmed": 5000000000,
    "immature": 0,
    "trusted_pending": 0,
    "untrusted_pending": 0
  }
}
>> wallet list_transactions
[
  {
    "confirmation_time": {
      "height": 1,
      "timestamp": 1690855159
    },
    "fee": 0,
    "received": 5000000000,
    "sent": 0,
    "transaction": null,
    "txid": "ae21741e8094f71d5bf8ae2db41d3e7d9c8f5d4a246913cc228934664903d0bd"
  }
]

note that the txid is from the 1st block examined in step 2. all is good so far.

get a new address from bdk-cli (note the issue also occurs even if the same address is used)

>> wallet get_new_address
{
  "address": "bcrt1q2dxwe0mpe2yvk9ak86v8qk5seav4ez4vq5z59ywulauexapzhf8qfhwcmq"
}

mine 1 block to the new address. block hash: 4a82362711babdf23c0c54e0c4ff060394b269473a27e009eae3311051f2c0e5
mine 100 blocks to unlock the subsidy as before.

# bitcoin-cli -regtest -rpcuser=<redacted> -rpcpassword=<redacted> -rpcwallet=<redacted> getblock 4a82362711babdf23c0c54e0c4ff060394b269473a27e009eae3311051f2c0e5
{
  "hash": "4a82362711babdf23c0c54e0c4ff060394b269473a27e009eae3311051f2c0e5",
  "confirmations": 101,
  "height": 102,
  "version": 536870912,
  "versionHex": "20000000",
  "merkleroot": "bc9744bd1a19b16319e417283528fac6474b6db745ca2d0c3a2cc8b8b5e9889d",
  "time": 1690855894,
  "mediantime": 1690855257,
  "nonce": 0,
  "bits": "207fffff",
  "difficulty": 4.656542373906925e-10,
  "chainwork": "00000000000000000000000000000000000000000000000000000000000000ce",
  "nTx": 1,
  "previousblockhash": "0a1cb26dde634c8a89b016e21e219ab543287c908062899bf5c19c4a780b782e",
  "nextblockhash": "6c6d41e8942b93cb0114115178538ea392b429eeb0b2e860946b569c8c27bc05",
  "strippedsize": 226,
  "size": 262,
  "weight": 940,
  "tx": [
    "bc9744bd1a19b16319e417283528fac6474b6db745ca2d0c3a2cc8b8b5e9889d"
  ]
}

expectated behavior:

we should expect to see transaction bc9744bd1a19b16319e417283528fac6474b6db745ca2d0c3a2cc8b8b5e9889d listed in the wallet transactions (and unspent) and a new confirmed balance of 10000000000.

actual behavior:

>> wallet sync
{}
>> wallet list_transactions
[
  {
    "confirmation_time": {
      "height": 1,
      "timestamp": 1690855159
    },
    "fee": 0,
    "received": 5000000000,
    "sent": 0,
    "transaction": null,
    "txid": "ae21741e8094f71d5bf8ae2db41d3e7d9c8f5d4a246913cc228934664903d0bd"
  }
]
>> wallet get_balance
{
  "satoshi": {
    "confirmed": 5000000000,
    "immature": 0,
    "trusted_pending": 0,
    "untrusted_pending": 0
  }
}

extra verification of the issue

so the question I had here is whether or not BDK was even seeing the second transaction. Maybe Fulcrum has a bug?

  1. exit bdk-cli repl
  2. cd ~/.bdk-bitcoin
  3. rm -rf * -- remove all sled databases

now, if I reopen the repl and sync again I should expect to see either:
A) 1 transaction and the second (index 1) address returned from wallet get_new_address
B) 2 transactions and the third (index 2) address returned from wallet get_new_address

$ bdk-cli --network regtest repl --server localhost:50001 --descriptor "wsh(multi(2,[25732241/84h/1h/0h/2h]tpubDFHnu4VcNjyj8VkUiAR5edPCRWJjrP2Mbar5BiLepxU23nEXo5mZmynzPc9h5wkqZD6bTWrsbTr1N4dryVjBMfgjYJBQUEDhzBU4NqHxQVu/0/*,[7737acb8]tpubD6NzVbkrYhZ4X3vCdevm3aFUdeSAWsbR4UhhdApbXnVUnvuwtq5NYqQ7zCkNhYG4JcPdGW4Wg4ZrVSYGpdoWsn337qmkP6CFgs7pZid12Sa/0/*,[dd7a3df3/84h/1h/0h/2h]tpubDEcxXkhzStbfzt9gPEbph3dGeR9uBMkr2rStYHcqxo9LwPDgAqbMgbYDJ8334MSesVVAwxcdYm4o7XJGjyDX6qNYEbTZov2gaMdGZjYFqXv/0/*))#00f5ljet"
>> wallet sync
{}
>> wallet get_balance
{
  "satoshi": {
    "confirmed": 5000000000,
    "immature": 0,
    "trusted_pending": 0,
    "untrusted_pending": 0
  }
}
>> wallet list_transactions
[
  {
    "confirmation_time": {
      "height": 102,
      "timestamp": 1690855894
    },
    "fee": 0,
    "received": 5000000000,
    "sent": 0,
    "transaction": null,
    "txid": "bc9744bd1a19b16319e417283528fac6474b6db745ca2d0c3a2cc8b8b5e9889d"
  }
]
>> wallet get_new_address
{
  "address": "bcrt1q87fgsgmq5tu7tuc73l93wa92t0ul6rfj7p5gwru69s2wjm2ychzswfmkmw"
}

note that I'm seeing 1 transaction and the index 2 address as the next unused address.
note that the 2nd transaction has replaced the 1st transaction in the list of transactions.

verify results:

in Rust:

fn main() -> Result<(), bdk::Error> {
  let wallet: Wallet<MemoryDatabase> = Wallet::new(
      "wsh(multi(2,[25732241/84h/1h/0h/2h]tpubDFHnu4VcNjyj8VkUiAR5edPCRWJjrP2Mbar5BiLepxU23nEXo5mZmynzPc9h5wkqZD6bTWrsbTr1N4dryVjBMfgjYJBQUEDhzBU4NqHxQVu/0/*,[7737acb8]tpubD6NzVbkrYhZ4X3vCdevm3aFUdeSAWsbR4UhhdApbXnVUnvuwtq5NYqQ7zCkNhYG4JcPdGW4Wg4ZrVSYGpdoWsn337qmkP6CFgs7pZid12Sa/0/*,[dd7a3df3/84h/1h/0h/2h]tpubDEcxXkhzStbfzt9gPEbph3dGeR9uBMkr2rStYHcqxo9LwPDgAqbMgbYDJ8334MSesVVAwxcdYm4o7XJGjyDX6qNYEbTZov2gaMdGZjYFqXv/0/*))#00f5ljet",
      None,
      Network::Regtest,
      MemoryDatabase::default(),
  ).unwrap();

  let address: bdk::wallet::AddressInfo = wallet.get_address(AddressIndex::Peek(0)).unwrap();
  println!("{:?}", address);
  let address: bdk::wallet::AddressInfo = wallet.get_address(AddressIndex::Peek(1)).unwrap();
  println!("{:?}", address);
  let address: bdk::wallet::AddressInfo = wallet.get_address(AddressIndex::Peek(2)).unwrap();
  println!("{:?}", address);

  println!("sync...");
  let client: Client = Client::new("127.0.0.1:50001").unwrap();
  let blockchain: ElectrumBlockchain = ElectrumBlockchain::from(client);
  wallet.sync(&blockchain, SyncOptions::default()).unwrap();

  let address: bdk::wallet::AddressInfo = wallet.get_address(AddressIndex::LastUnused).unwrap();
  println!("{:?}", address);

  let vec = wallet.list_transactions(false).unwrap();
  vec.iter().for_each(|tx| println!("{:?}", tx));

  Ok(())
}
$ cargo run
AddressInfo { index: 0, address: bcrt1q6g0mhnwcssfulc39as04k4xazaxt2s6eg0q6yj4uylzzcfy73n3sw8h3pw, keychain: External }
AddressInfo { index: 1, address: bcrt1q2dxwe0mpe2yvk9ak86v8qk5seav4ez4vq5z59ywulauexapzhf8qfhwcmq, keychain: External }
AddressInfo { index: 2, address: bcrt1q87fgsgmq5tu7tuc73l93wa92t0ul6rfj7p5gwru69s2wjm2ychzswfmkmw, keychain: External }
sync...
AddressInfo { index: 2, address: bcrt1q87fgsgmq5tu7tuc73l93wa92t0ul6rfj7p5gwru69s2wjm2ychzswfmkmw, keychain: External }
TransactionDetails { transaction: None, txid: bc9744bd1a19b16319e417283528fac6474b6db745ca2d0c3a2cc8b8b5e9889d, received: 5000000000, sent: 0, fee: Some(0), confirmation_time: Some(BlockTime { height: 102, timestamp: 1690855894 }) }

Build environment

  • BDK tag/commit: 0.28.0
  • bdk-cli version: 0.27.1
  • OS+version: 6.1.38-1-MANJARO
  • Rust/Cargo version: 1.69.0
  • Rust/Cargo target: x86_64-unknown-linux-gnu

Additional context

@turkycat turkycat added the bug Something isn't working label Aug 1, 2023
@notmandatory notmandatory moved this to Todo in BDK Maintenance Aug 15, 2023
@nondiremanuel nondiremanuel added this to the 0.29.0 milestone Aug 15, 2023
@danielabrozzoni danielabrozzoni self-assigned this Aug 22, 2023
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Aug 22, 2023
We would previously insert just one coinbase transaction in the database
if we caught multiple in the same sync.
When we sync with electrum, before committing to the database, we remove
from the update conflicting transactions, using the
`make_txs_consistent` function. This function considers two txs to be
conflicting if they spend from the same outpoint - but every coinbase
transaction spends from the same outpoint!
Here we make sure to avoid filtering out coinbase transactions, by
adding a check on the txid just before we do the filtering.

Fixes bitcoindevkit#1051
danielabrozzoni added a commit to danielabrozzoni/bdk that referenced this issue Aug 22, 2023
We would previously insert just one coinbase transaction in the database
if we caught multiple in the same sync.
When we sync with electrum, before committing to the database, we remove
from the update conflicting transactions, using the
`make_txs_consistent` function. This function considers two txs to be
conflicting if they spend from the same outpoint - but every coinbase
transaction spends from the same outpoint!
Here we make sure to avoid filtering out coinbase transactions, by
adding a check on the txid just before we do the filtering.

Fixes bitcoindevkit#1051
@danielabrozzoni
Copy link
Member

Thank you so much for the detailed explanation of the problem! Indeed it is a bug (and it is pretty crazy that no one noticed it before). #1090 should fix.

@danielabrozzoni danielabrozzoni linked a pull request Aug 22, 2023 that will close this issue
5 tasks
notmandatory added a commit that referenced this issue Aug 22, 2023
530ba36 ci: fix msrv dependency versions for reqest and h2 (Daniela Brozzoni)
7a359d5 fix(electrum): Don't ignore multiple coinbase txs (Daniela Brozzoni)

Pull request description:

  We would previously insert just one coinbase transaction in the database if we caught multiple in the same sync.
  When we sync with electrum, before committing to the database, we remove from the update conflicting transactions, using the `make_txs_consistent` function. This function considers two txs to be conflicting if they spend from the same outpoint - but every coinbase transaction spends from the same outpoint!
  Here we make sure to avoid filtering out coinbase transactions, by adding a check on the txid just before we do the filtering.

  Fixes #1051

  ### Changelog notice

  - Fix a bug when syncing coinbase utxos on electrum

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### Bugfixes:

  * [x] I've added tests to reproduce the issue which are now passing
  * [x] I'm linking the issue being fixed by this PR

ACKs for top commit:
  notmandatory:
    tACK 530ba36

Tree-SHA512: ebbc6af86d4433ac4083212033a23f183d109641db345cc06ab4d506995ab71657761351c03772462ab4ff0d081226ecc97f1042490194aaf8661914cbeb72cb
@github-project-automation github-project-automation bot moved this from Todo to Done in BDK Maintenance Aug 22, 2023
@turkycat
Copy link
Author

🥇 thanks so much for the quick fix @danielabrozzoni and team

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants