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

Change not being detected by wallet #95

Closed
i5hi opened this issue May 26, 2022 · 13 comments
Closed

Change not being detected by wallet #95

i5hi opened this issue May 26, 2022 · 13 comments
Labels
bug Something isn't working

Comments

@i5hi
Copy link
Contributor

i5hi commented May 26, 2022

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

  • Account 0 Deposit Descriptor
wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/0/*)
  • Account 0 Change Descriptor
wpkh([8099ce1e/84h/1h/0h]tpubDCBjCC5aZ6wXLtZMSJDkBYZ3AFuors2YzzBhD5ZqP3uPqbzzH5YjD2CA9HDhUYNhrqq67v4XAN93KSbSL4bwa5hEvidkFuj7ycWA7EYzp41/1/*)
  • bdk-cli get_balance

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
Copy link
Contributor Author

i5hi commented May 26, 2022

bdk-cli --version                                                                                                                                                                     
BDK CLI 0.5.0

@i5hi
Copy link
Contributor Author

i5hi commented May 26, 2022

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
Copy link
Contributor Author

i5hi commented May 26, 2022

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
Copy link
Contributor Author

i5hi commented May 26, 2022

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
Copy link
Contributor Author

i5hi commented May 26, 2022

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

@notmandatory notmandatory added the bug Something isn't working label May 26, 2022
@rajarshimaitra
Copy link
Contributor

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
Copy link
Contributor Author

i5hi commented May 28, 2022

really strange...

@notmandatory
Copy link
Member

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
Copy link
Member

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

@notmandatory notmandatory moved this to In Progress in BDK-CLI Roadmap May 28, 2022
@notmandatory notmandatory moved this from In Progress to Todo in BDK-CLI Roadmap May 28, 2022
@i5hi
Copy link
Contributor Author

i5hi commented May 29, 2022

Cheers @notmandatory!

@i5hi i5hi closed this as completed May 29, 2022
Repository owner moved this from Todo to Done in BDK-CLI Roadmap May 29, 2022
@i5hi
Copy link
Contributor Author

i5hi commented May 29, 2022

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
Copy link
Member

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.

@i5hi
Copy link
Contributor Author

i5hi commented May 29, 2022

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!

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

No branches or pull requests

3 participants