Listunspent txout address filtering and listaddressgroupings #1672

1. 
   gmaxwell commented at 6:28 PM on August 13, 2012: contributor

   This applies on top of the coincontrol listaddressgroupings patch and makes finding eligible outputs from the groups returned by listaddressgroupings possible.

2. 
   BitcoinPullTester commented at 9:18 PM on August 13, 2012: none

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

3. 
   sipa commented at 12:47 PM on August 17, 2012: member

   I'd like to keep the Base58 dependency (CBitcoinAddress, storing destinations as strings) out of the core wallet code, and use the internal representation there (CTxDestination); that should be more efficient as well. Can you make the CWallet:: methods work with CTxDestination, and have the RPC code do the transformation to strings?

4. 
   gmaxwell commented at 4:48 PM on August 17, 2012: contributor

   Sounds totally reasonable.

5. 
   gmaxwell commented at 5:47 PM on August 20, 2012: contributor

   Rebased, switched to CTxDestination, added some help text.

   I have some code here from a fork that adds some filter argument to listaddressgroupings (e.g. to filter groups by available value), any opinions on it? (I'd have to go cut it out, as the fork was a failed effort to generalize the grouping code that I gave up one once I realized I was adding an email client to it)

6. in src/rpcrawtransaction.cpp:None in 69558ba930 outdated

    143 | @@ -144,24 +144,40 @@ Value getrawtransaction(const Array& params, bool fHelp)
    144 |  
    145 |  Value listunspent(const Array& params, bool fHelp)
    146 |  {
    147 | -    if (fHelp || params.size() > 2)
    148 | +    if (fHelp || params.size() > 3)
    149 |          throw runtime_error(
    150 | -            "listunspent [minconf=1] [maxconf=999999]\n"
    151 | +            "listunspent [minconf=1] [maxconf=9999999] ['addr1','addr2',...]\n"
   

   ---

   ---

   

   gavinandresen commented at 6:26 PM on August 20, 2012:

   Nit: createrawtransaction help uses 'address' and double-quotes: createrawtransaction [{"txid":txid,"vout":n},...] {address:amount,...}

   listunspent help should follow: "listunspent [minconf=1] [maxconf=9999999] ["address",...]\n"

7. 
   gmaxwell commented at 6:35 PM on August 20, 2012: contributor

   Denitted.

8. 
   BitcoinPullTester commented at 5:33 AM on August 21, 2012: none

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

9. 
   sipa commented at 2:26 PM on August 21, 2012: member

   Apart from that, ACK.

10. 
    sipa commented at 11:51 AM on August 22, 2012: member

    I don't really have a suggestion for making it less ambiguous, and it's a problem that doesn't occur only here.

11. 
    gmaxwell commented at 7:19 PM on August 22, 2012: contributor

    Perhaps [[optional_array_member,...]] ?

12. 
    gmaxwell commented at 8:39 PM on August 22, 2012: contributor

    Rebased for the RPC reorg.

13. 
    BitcoinPullTester commented at 5:37 AM on August 23, 2012: none

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

14. Add address groupings RPC from the coincontrol patches.

    Signed-off-by: Gregory Maxwell <greg@xiph.org>

    22dfd73598
15. Add txout address filtering to listunspent.

    This applies on top of the coincontrol listaddressgroupings patch
    and makes finding eligible outputs from the groups returned
    by listaddressgroupings possible.

    92735bca31
16. in src/wallet.cpp:None in 509b0116b0 outdated

    1670 | +        CWalletTx *pcoin = &walletEntry.second;
    1671 | +
    1672 | +        if (!pcoin->IsFinal() || !pcoin->IsConfirmed())
    1673 | +            continue;
    1674 | +
    1675 | +        if (pcoin->IsCoinBase() && pcoin->GetBlocksToMaturity() > 0)
    

    ---

    ---

    

    luke-jr commented at 7:13 AM on August 24, 2012:

    I'm not sure it makes sense to exclude unconfirmed/immature transactions from the grouping algorithm?

    ---

    

    gmaxwell commented at 7:19 AM on August 24, 2012:

    Right— I agreed. I removed it once; but with all the rewrites and rebases I forgot I'd done that.

17. 
    luke-jr commented at 7:14 AM on August 24, 2012: member

    Besides comment (on excluding unconfirmed stuff from algo), I didn't find any obvious issues. However, I'm not familiar enough with the rawtx stuff that I place any significant meaning on my review.

18. Change CWallet addressgrouping to use CTxDestination instead of strings.

    This is cleanup for the listaddressgroupings code. Also add some
    real help text.

    b1093efa83
19. gmaxwell referenced this in commit c68c4bc7a4 on Aug 24, 2012
20. gmaxwell merged this on Aug 24, 2012
21. gmaxwell closed this on Aug 24, 2012

    ---

22. suprnurd referenced this in commit 296cfd2efa on Dec 5, 2017
23. DrahtBot locked this on Sep 8, 2021

Contributors

gmaxwellBitcoinPullTestersipagavinandresenluke-jr

Milestone
0.7.0