bitcoindevkit / bdk-cli

A CLI wallet library and REPL tool to demo and test the BDK library
Other
108 stars 64 forks source link

Change not being detected by wallet #95

Closed i5hi closed 2 years ago

i5hi commented 2 years ago

Issue

Change from a wallet is not being picked up in get_balance when both change and deposit descriptors are used together. When Change descriptor is used alone, funds are detected.

Reproduction Steps

wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)
wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)

Ignoring sync

bdk-cli wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)" -c "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" get_balance 
{
  "satoshi": 1253
}
bdk-cli wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" get_balance
{
  "satoshi": 1154933
}
i5hi commented 2 years ago
bdk-cli --version                                                                                                                                                                     
BDK CLI 0.5.0
i5hi commented 2 years ago

We first spotted this bug in stackmate mobile and used bdk-cli to confirm that the error was not in our implementation.

Also, stackmate just recently (a week to 10 days ago) updated bdk from 0.13.0 to 0.18.0. We didn't, notice this bug previously OR it might have slipped us since we always just expected get_balance to be displaying the correct result.

The first pointer that made us spot something strange was this transaction:

 {
    "confirmation_time": {
      "height": 2247499,
      "timestamp": 1653548350
    },
    "fee": 7059,
    "received": 0,
    "sent": 91933,
    "transaction": null,
    "txid": "bd8be8ae6c1ec5ee272bab075af8df810a1686ef233683de7edd7e8dd3ca7cf5"
  }

The actual amount we sent was 12k sats. Which we received in the other wallet. But this sending wallet, isn't recognising its change and marking it as a send to external.

i5hi commented 2 years ago

If we look into this tx: https://mempool.bullbitcoin.com/testnet/tx/bd8be8ae6c1ec5ee272bab075af8df810a1686ef233683de7edd7e8dd3ca7cf5 tb1q883xtcfqlw0744rwx3c583lujhn8mpfnr35pvx (OUR CHANGE) ‎0.00072338 tBTC
tb1q5ks04m6409n4v...kn66hp2nyqd3w2laqa (RECIPIENT) ‎0.00012536 tBTC

i5hi commented 2 years ago

Just to confirm the above change address is from the same change descriptor:

bdk-cli wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" get_new_address
{
  "address": "tb1q883xtcfqlw0744rwx3c583lujhn8mpfnr35pvx"
}
i5hi commented 2 years ago

I also noticed that while trying different bdk-cli versions, sync followed by get_balance would initially show balance as 0. If I run get_address after it then get_balance again, then the balance updates :S

Will open an other issue for that if I notice it happening more often. So far, this happened twice, with version 0.3.0 and 0.5.0

Edit: Opened issue here

rajarshimaitra commented 2 years ago

I have tried to reproduce this problem in regtest.. Apparently the issue is only occurring for this particular descriptor.. I am not being able to generate balance even when I am sending txs..

$ ./target/debug/bdk-cli -n regtest wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)" -c "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" get_new_address
{
  "address": "bcrt1qdxlep73m9nte7yaxp6mw60fargpjp2qmc0xlea"
}

This creates a watch only wallet named lsshgur8wy4k4fnl in core..

$ bitcoin-cli listwalletdir
{
  "wallets": [
    {
      "name": "lsshgur8wy4k4fnl"
    },
    {
      "name": "test"
    }
  ]
}

Now sending transaction to the derived address

$ bitcoin-cli -rpcwallet=test sendtoaddress bcrt1qy6q3xr40lphlzx435lqnl68un3jqhzjgjf2nwy 2
ad3e6a27981f6bb9518de20745c2ec798c5e0af29b9e125570a754577de528c1

The regtest log is only showing outgoing transaction corresponding to the test wallet

2022-05-27T15:54:39Z [test] Fee non-grouped = 1410, grouped = 1410, using grouped
2022-05-27T15:54:39Z [test] CommitTransaction:
CTransaction(hash=ad3e6a2798, ver=2, vin.size=1, vout.size=2, nLockTime=314)
    CTxIn(COutPoint(b188b23379, 0), scriptSig=, nSequence=4294967294)
    CScriptWitness(304402202f80c33ecb4dddec11e49e40803307acb14d31d74dddde81fd5c94b31df647d6022038ab5ad78289db3a86619d1275066eb61865b29fee2d3d382f78e9766676ccde01, 03d493217e94a3382537d79b392b4bcbab72cb6a2b759544b8a82e103db6890415)
    CTxOut(nValue=2.00000000, scriptPubKey=00142681130eaff86ff11ab1a7c13f)
    CTxOut(nValue=20.99997180, scriptPubKey=0014ae784df84229ccadad3a1eb6e4)
2022-05-27T15:54:39Z [test] AddToWallet ad3e6a27981f6bb9518de20745c2ec798c5e0af29b9e125570a754577de528c1  newupdate
2022-05-27T15:54:39Z [test] Submitting wtx ad3e6a27981f6bb9518de20745c2ec798c5e0af29b9e125570a754577de528c1 to mempool for relay
2022-05-27T15:54:39Z [test] AddToWallet ad3e6a27981f6bb9518de20745c2ec798c5e0af29b9e125570a754577de528c1  

Get balance from bdk is then showing nothing

$ ./target/debug/bdk-cli -n regtest wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)" -c "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" get_balance
{
  "satoshi": 0
}

And when using bitcoin-cli to get the balance of that wallet, its still showing zero..

 $ bitcoin-cli -rpcwallet=lsshgur8wy4k4fnl getbalance
0.00000000

I am not exactly sure what is going on, but this issue seems to be related particularly with that descriptor..

i5hi commented 2 years ago

really strange...

notmandatory commented 2 years ago

I think this is a stop gap address issues, ie. the number of unused addresses looked at during syncing before it stops looking for new transactions. I was able to find all coins by using sync --max_addresses 200:

rm -rf ~/.bdk-bitcoin 
bdk-cli wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)" -c "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" sync 
{}
bdk-cli wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)" -c "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" get_balance
{
  "satoshi": 1253
}
bdk-cli wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)" -c "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" sync --max_addresses 200
{}
bdk-cli wallet -d "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)" -c "wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)" get_balance
{
  "satoshi": 1156186
}
notmandatory commented 2 years ago

This often happens when using a descriptor for testing and generate many new addresses that don't receive any new transactions.

i5hi commented 2 years ago

Cheers @notmandatory!

i5hi commented 2 years ago

This has actually made me aware of another issue that we could potentially bump into on stackmate.

We currently allow users to index addresses manually. We didnt initially have this but we added it just for a case with imported wallets. In a case where i have a wallet where the last used index is 6, and i have handed out addresses 7, 8, 9. When i import this wallet, it will start at address 6 and since i know i have used 7, 8, 9 (although not recieved funds) i should be able to increment up to that.

I think now, we should limit how many indexes the user can rotate, to prevent them from just incrementing arbitratily and go beyong the index_gap.

notmandatory commented 2 years ago

A related PR you might find interesting is: https://github.com/bitcoindevkit/bdk/pull/546

It gives more control to find unused addresses and fill in gaps. Once this is merged and released in BDK it wouldn't be hard to expose this functionality in bdk-ffi and then the other languages.

i5hi commented 2 years ago

A related PR you might find interesting is: bitcoindevkit/bdk#546

It gives more control to find unused addresses and fill in gaps. Once this is merged and released in BDK it wouldn't be hard to expose this functionality in bdk-ffi and then the other languages.

Nice! This is a really useful addition!

Thanks!