wallet: Fix segfault in CreateWalletFromFile #16796

1. 
   MarcoFalke commented at 6:25 PM on September 3, 2019: member

   Comes with a test to aid review. The test should fail without the fix to bitcoind

   The following CreateWalletFromFile issues are fixed:

   • walletFile refers to freed memory and will thus corrupt the debug.log and/or crash the node if read
   • WalletParameterInteraction was moved to CreateWalletFromFile and WalletInit::ParameterInteraction without updating the documentation
2. wallet: Fix documentation around WalletParameterInteraction faa13539d5
3. test: Print both messages on failure in assert_raises_message fab3c34412
4. wallet: Fix segmentation fault in CreateWalletFromFile fa734603b7
5. MarcoFalke added the label Wallet on Sep 3, 2019
6. MarcoFalke added this to the milestone 0.19.0 on Sep 3, 2019
7. 
   promag commented at 12:46 AM on September 6, 2019: member

   ACK fa734603b78ba31ebf0da5d2dbe87386eafff01a.

   fab3c34412379598b812631e3c123e9467cdc485 is a nice addition.

8. 
   darosior commented at 11:32 AM on September 8, 2019: member

   ACK fa734603b78ba31ebf0da5d2dbe87386eafff01a Tested locally.

9. in test/functional/wallet_multiwallet.py:36 in fa734603b7

     28 | @@ -27,6 +29,13 @@ def set_test_params(self):
     29 |      def skip_test_if_missing_module(self):
     30 |          self.skip_if_no_wallet()
     31 |  
     32 | +    def add_options(self, parser):
     33 | +        parser.add_argument(
     34 | +            '--data_wallets_dir',
     35 | +            default=os.path.join(os.path.dirname(os.path.realpath(__file__)), 'data/wallets/'),
     36 | +            help='Test data with wallet directories (default: %(default)s)',
   

   ---

   ---

   

   meshcollider commented at 11:43 PM on September 8, 2019:

   I'm not 100% fluent in python and haven't tested this but don't you need to use e.g. % locals() at the end to fill the %(default)s?

   ---

   

   MarcoFalke commented at 11:05 AM on September 9, 2019:

   This is done by the argparse module. It can be tested with

   $ ./test/functional/wallet_multiwallet.py --help|tail -4
     --data_wallets_dir DATA_WALLETS_DIR
                           Test data with wallet directories (default: /home/marc
                           o/workspace/btc_bitcoin_core/test/functional/data/wall
                           ets/)
   

10. 
    meshcollider commented at 11:32 AM on September 9, 2019: contributor

    LGTM, code-read ACK fa734603b78ba31ebf0da5d2dbe87386eafff01a

11. meshcollider merged this on Sep 9, 2019
12. meshcollider closed this on Sep 9, 2019

    ---

13. 
    ryanofsky commented at 3:02 PM on September 9, 2019: member

    test/functional/data/wallets/high_minversion/wallet.dat

    I think if we're going to add binary test blobs like this, they should be accompanied with comments saying how they are generated and intended to be maintained. For example, this information could go in accompanying README.md or generate.sh files.

    Without this information, as code is updated in the future, and especially if more binary blobs are added, we're going to be faced with unpleasant choices of:

    1. Deleting tests and potentially losing important test coverage when changes are made.
    2. Or having to add ugly workarounds to production or test code to maintain compatibility with test blobs.
    3. Or having to do historical research and archaeology to figure out how to update test blobs.

    Better to just record upfront what's in the blobs and how they should be treated. Would be nice to have a followup PR adding this.

14. MarcoFalke deleted the branch on Sep 10, 2019
15. sidhujag referenced this in commit 304bc5bf0d on Sep 10, 2019
16. MarcoFalke referenced this in commit 796b713633 on Sep 16, 2019
17. sidhujag referenced this in commit b58c25bd3b on Sep 16, 2019
18. deadalnix referenced this in commit 3312619d73 on Jul 28, 2020
19. PastaPastaPasta referenced this in commit d93549e98d on Jun 27, 2021
20. PastaPastaPasta referenced this in commit f378f8fedf on Jun 28, 2021
21. PastaPastaPasta referenced this in commit e14a37280e on Jun 29, 2021
22. PastaPastaPasta referenced this in commit 0ce0726e9e on Jul 1, 2021
23. PastaPastaPasta referenced this in commit 67eb67a409 on Jul 1, 2021
24. PastaPastaPasta referenced this in commit d6c9235bec on Jul 12, 2021
25. PastaPastaPasta referenced this in commit 008ecf5394 on Sep 11, 2021
26. PastaPastaPasta referenced this in commit 939f65c2f8 on Sep 11, 2021
27. PastaPastaPasta referenced this in commit 8c646b7211 on Sep 12, 2021
28. PastaPastaPasta referenced this in commit 1baebcf326 on Sep 12, 2021
29. PastaPastaPasta referenced this in commit aaab554030 on Sep 12, 2021
30. PastaPastaPasta referenced this in commit a3cae5c165 on Sep 14, 2021
31. PastaPastaPasta referenced this in commit 5506e7d268 on Sep 14, 2021
32. kittywhiskers referenced this in commit 3360ae9a73 on Nov 6, 2021
33. kittywhiskers referenced this in commit 9485e1b85a on Dec 5, 2021
34. kittywhiskers referenced this in commit 8d16db5cd1 on Dec 5, 2021
35. Munkybooty referenced this in commit b82843d3b7 on Dec 7, 2021
36. kittywhiskers referenced this in commit 2acdcec063 on Dec 12, 2021
37. kittywhiskers referenced this in commit fc6e35060d on Dec 12, 2021
38. Munkybooty referenced this in commit 75f39dd362 on Dec 15, 2021
39. Munkybooty referenced this in commit b35438a8a9 on Dec 15, 2021
40. DrahtBot locked this on Dec 16, 2021

Contributors

MarcoFalkepromagdarosiormeshcolliderryanofsky

Milestone
0.19.0