tests: Ensure sorted/multi_a descriptors always generate different addrs #24490

1. 
   achow101 commented at 11:01 AM on March 7, 2022: member

   Sometimes the multi_a and sortedmulti_a descriptors will produce some of the same addresses in the tests. This causes the wallets to start generating addresses at a different index as they detect that one of the addresses is used. This subsequently causes a test failure.

   To avoid this problem, use descriptors that will produce unique addresses by putting one of the multi_a in a different branch.

2. achow101 cross-referenced this on Mar 7, 2022 from issue Add (sorted)multi_a descriptor for k-of-n multisig inside tr by sipa
3. achow101 force-pushed on Mar 7, 2022
4. achow101 force-pushed on Mar 7, 2022
5. tests: Ensure sorted/multi_a descriptors always generate different addrs

   Sometimes the multi_a and sortedmulti_a descriptors will produce some of
   the same addresses in the tests. This causes the wallets to start
   generating addresses at a different index as they detect that one of
   the addresses is used. This subsequently causes a test failure.
   
   To avoid this problem, use descriptors that will produce unique
   addresses by putting one of the multi_a in a different branch.

   db27ac9354
6. achow101 force-pushed on Mar 7, 2022
7. 
   theStack commented at 11:37 AM on March 7, 2022: contributor

   Concept ACK

8. fanquake added the label Tests on Mar 7, 2022
9. theStack approved
10. 
    theStack commented at 1:19 PM on March 7, 2022: contributor

    Tested ACK db27ac935480aeec40a1cfc9d5f10a48be55d61b

    On master, generating the same addresses causing the problem described in the OP can be reproduced with the following patch (determined in the course of investigating the CI failure #24486 (comment), unrelated to the linked PR):

    diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py
    index 54c992852..5443f815a 100755
    --- a/test/functional/wallet_taproot.py
    +++ b/test/functional/wallet_taproot.py
    @@ -333,6 +333,10 @@ class WalletTaprootTest(BitcoinTestFramework):
         def do_test(self, comment, pattern, privmap, treefn):
             nkeys = len(privmap)
             keys = self.rand_keys(nkeys * 4)
    +
    +        if nkeys == 2:  # these keys will result in same addresses for sorted_a/multi_a descriptors
    +            keys = [KEYS[i] for i in [11, 6, 0, 1, 2, 3, 4, 5]]
    +
             self.do_test_addr(comment, pattern, privmap, treefn, keys[0:nkeys])
             self.do_test_sendtoaddress(comment, pattern, privmap, treefn, keys[0:nkeys], keys[nkeys:2*nkeys])
             self.do_test_psbt(comment, pattern, privmap, treefn, keys[2*nkeys:3*nkeys], keys[3*nkeys:4*nkeys])
    

    .....
    2022-03-07T13:12:18.658000Z TestFramework (INFO): Testing tr(H,multi_a(1,XPUB,XPRV)) address derivation
    2022-03-07T13:12:18.914000Z TestFramework (ERROR): Assertion failed
    Traceback (most recent call last):
      File "/home/honey/bitcoin/test/functional/test_framework/test_framework.py", line 132, in main
        self.run_test()
      File "./test/functional/wallet_taproot.py", line 435, in run_test
        self.do_test(
      File "./test/functional/wallet_taproot.py", line 340, in do_test
        self.do_test_addr(comment, pattern, privmap, treefn, keys[0:nkeys])
      File "./test/functional/wallet_taproot.py", line 251, in do_test_addr
        assert_equal(addr_g, addr_r)
      File "/home/honey/bitcoin/test/functional/test_framework/util.py", line 51, in assert_equal
        raise AssertionError("not(%s)" % " == ".join(str(arg) for arg in (thing1, thing2) + args))
    AssertionError: not(bcrt1px3pnh5n8npeqr3cv0aq6dvqpcsujrgksfd6hup8dpa62z3m9jf9s3sk3ve == bcrt1pn2xq83tt55vfsxtwhkx96sanchkjzwg6yqxuewvn6emkfyqnyp0qe5cteg)
    2022-03-07T13:12:18.987000Z TestFramework (INFO): Stopping nodes
    
    .....
    

    Running the PR branch with this patch succeeds.

11. theStack cross-referenced this on Mar 7, 2022 from issue wallet: refactor: dedup sqlite blob binding by theStack
12. 
    ajtowns commented at 5:30 PM on March 7, 2022: contributor

    ACK db27ac935480aeec40a1cfc9d5f10a48be55d61b

    100 runs with no more failures and explanation makes sense

13. fanquake requested review from instagibbs on Mar 7, 2022
14. fanquake cross-referenced this on Mar 7, 2022 from issue ci: failure in wallet_taproot.py by fanquake
15. MarcoFalke merged this on Mar 9, 2022
16. MarcoFalke closed this on Mar 9, 2022

    ---

17. sidhujag referenced this in commit 0d66767abc on Mar 9, 2022
18. bitcoin locked this on Mar 9, 2023

Review Requested
instagibbs