In listaddressgroupings push down the IsMine check to run on each input. #1872

1. 
   gmaxwell commented at 5:42 PM on September 27, 2012: contributor

   This avoids a potential crash when trying to read the scrippubkeys on transactions where the first input IsMine but some of the rest are not when running listaddressgroupings.

2. 
   BitcoinPullTester commented at 11:09 PM on September 27, 2012: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/41906a3ea8322716d6391e5afde90dc81f017651 for binaries and test log.

3. 
   sipa commented at 1:58 PM on September 28, 2012: member

   Haven't tested, but code changes look ok.

4. 
   gavinandresen commented at 9:02 PM on October 1, 2012: contributor

   Sanity testing on a testnet-in-a-box "hell wallet", I get an extra empty array:

   Old:

   gavin$ head /tmp/f2
   [
       [
           [
               "mmU33ZKwUkSoVSx8bHUnf2PmNosDMWvxsU",
               17.93950000
           ],
           [
               "mzGzLeJA2up3VL4gW32x5hT5YvEaA7SnBh",
               0.00000000
           ],
   

   New:

   gavin$ head /tmp/f1
   [
       [
       ],
       [
           [
               "mmU33ZKwUkSoVSx8bHUnf2PmNosDMWvxsU",
               17.93950000
           ],
           [
               "mzGzLeJA2up3VL4gW32x5hT5YvEaA7SnBh",
   

5. 
   gmaxwell commented at 1:00 PM on October 2, 2012: contributor

   @gavinandresen Ah. Thanks. I'm pretty sure thats just from adding an empty group after encountering a transaction where it can't add any linked ins or outs. Pushing a revised patch that I expect will fix that (though I don't have a reproduction setup right now). It might be helpful to encrypt that wallet with some impossible key and post it.

6. 
   gavinandresen commented at 1:38 PM on October 2, 2012: contributor

   It is a testnet-in-a-box wallet, so it is not sensitive.

   Wallet and blockchain are at: http://www.skypaint.com/bitcoin/gavin_testnetbox_wallet.tar.gz

7. 
   gmaxwell commented at 2:56 PM on October 4, 2012: contributor

   Indeed, the empty group is gone with the latest update.

8. 
   jgarzik commented at 4:52 PM on October 9, 2012: contributor

   ACK

9. 
   BitcoinPullTester commented at 7:18 PM on October 20, 2012: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/f00b335d53380e10b528d3a4e3bf663624eeb61f for binaries and test log.

10. in src/wallet.cpp:None in f00b335d53 outdated

    1695 | +            if (any_mine)
    1696 | +            {
    1697 | +               BOOST_FOREACH(CTxOut txout, pcoin->vout)
    1698 | +                   if (IsChange(txout))
    1699 | +                   {
    1700 | +                       CWalletTx tx = mapWallet[pcoin->vin[0].prevout.hash];
    

    ---

    ---

    

    sipa commented at 11:26 PM on October 28, 2012:

    Any reason to refer to vin[0] specifically?

    ---

    

    sipa commented at 7:36 PM on November 21, 2012:

    Or even better, what is that 'tx' variable doing there at all?

11. 
    gavinandresen commented at 5:31 PM on December 12, 2012: contributor

    ACK if you remove the CWalletTx tx = .... dead line of code that sipa pointed out.

12. In listaddressgroupings push down the IsMine check to run on each input.

    This avoids a potential crash when trying to read the scrippubkeys on
    transactions where the first input IsMine but some of the rest are not
    when running listaddressgroupings.

    a3fad2119b
13. 
    gmaxwell commented at 12:38 PM on December 14, 2012: contributor

    Yea, dunno how that made it into there.. cruft from an earlier version. Sorry about that. Fixed and tested.

14. 
    BitcoinPullTester commented at 3:42 AM on December 16, 2012: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/a3fad2119bcf4638ed137fc5cddc2f34a3bd1dc2 for binaries and test log.

15. 
    Diapolo commented at 10:09 AM on January 11, 2013: none

    This one is lingering around for quite some time, I count 2 ACKs, any reason not to merge it or does it need a rebase?

16. gmaxwell referenced this in commit d40c164369 on Jan 22, 2013
17. gmaxwell merged this on Jan 22, 2013
18. gmaxwell closed this on Jan 22, 2013

    ---

19. laudney referenced this in commit 5c17cefd9a on Mar 19, 2014
20. KolbyML referenced this in commit 2f4e1caac1 on Dec 5, 2020
21. jamescowens referenced this in commit 86f14fe82c on Jul 18, 2021
22. jamescowens referenced this in commit dd9d8cc0a3 on Jul 18, 2021
23. DrahtBot locked this on Sep 8, 2021

Contributors

gmaxwellBitcoinPullTestersipagavinandresenjgarzikDiapolo

Milestone
0.8.0