btcsuite / btcwallet

A secure bitcoin wallet daemon written in Go (golang)
ISC License
1.15k stars 590 forks source link

waddrmgr scope incorrect,Possible non-compliance with BIP #916

Closed EthanShang8989 closed 8 months ago

EthanShang8989 commented 8 months ago
    bucket, _ := tx.CreateTopLevelBucket(waddrmgrNamespaceKey)
    err = waddrmgr.Create(bucket, key, pubPassphrase, privPassphrase, &chaincfg.TestNet3Params, nil, time.Time{})
    mgr, err := waddrmgr.Open(bucket, pubPassphrase, &chaincfg.TestNet3Params)
    scopedMgr, err = mgr.NewScopedKeyManager(bucket, waddrmgr.KeyScopeBIP0086, 
        waddrmgr.ScopeAddrMap[waddrmgr.KeyScopeBIP0086])
    fmt.Println("Scope", scopedMgr.Scope())
//Scope m/86'/0'

I think it should be m/86'/1'/0' to include an additional component to indicate whether it's for the mainnet or testnet. Otherwise, it may not be very compatible with mainstream wallets (I am using Sparrow).

guggero commented 8 months ago

Yes, you are correct. Not taking into account testnet/regtest/signet is incorrect. Though I think this particular inaccuracy has been there since the beginning. So it can't just be changed without breaking all existing wallets. You can fix it quite easily in your case:

    bip86Testnet := waddrmgr.KeyScopeBIP0086
    bip86Testnet.Coin = chaincfg.TestNet3Params.HDCoinType
    scopedMgr, err = mgr.NewScopedKeyManager(bucket, bip86Testnet, waddrmgr.ScopeAddrMap[waddrmgr.KeyScopeBIP0086])
EthanShang8989 commented 8 months ago

Currently, I mainly want to see the addresses from my Sparrow in the wallet's address section. The modification method you suggested seems unable to increase the path depth. My address path might be m/86'/1'/0'/0/0, but it feels like the path in the btcwallet might be m/86'/1'/0/0. This might be the reason why the addresses are different.

        externalAddresses, err := scopedMgr.NextExternalAddresses(bucket, 0, 1)
        fmt.Println("address: ", externalAddresses[0].Address().EncodeAddress())
guggero commented 8 months ago

I can't seem to reproduce this. I just tried it out with a random seed (tprv8ZgxMBicQKsPe1bubXi8MBHg2QrkPmuaAWGoRtSSiuF5catcWbEXEGnBFJhvoi5uhDjDPRmTPMuPg1vtVBEovJkB54E3QGbkYTYDKMeS7JX) and I got the first external address as tb1plx5fsh2m6fkttg9dqcsk6uq5vh98ynfsp564hrygjntxk42vjmfqxhwj0q which seems to match Sparrow. Can you try your code with the seed above and see what result you get?

EthanShang8989 commented 8 months ago

I can't seem to reproduce this. I just tried it out with a random seed (tprv8ZgxMBicQKsPe1bubXi8MBHg2QrkPmuaAWGoRtSSiuF5catcWbEXEGnBFJhvoi5uhDjDPRmTPMuPg1vtVBEovJkB54E3QGbkYTYDKMeS7JX) and I got the first external address as tb1plx5fsh2m6fkttg9dqcsk6uq5vh98ynfsp564hrygjntxk42vjmfqxhwj0q which seems to match Sparrow. Can you try your code with the seed above and see what result you get?

I tried again and it worked. Thank you for your answer. Now I'm considering whether to synchronize wallet information using a third-party browser API instead of using RPC. Maintaining a full node feels a bit difficult