Refactor: Create CCoinsViewEfficient interface for CCoinsViewCache #5747

1. 
   jtimon commented at 0:54 am on February 4, 2015: contributor

   To move AreInputsStandard() and its consensus homologous to policy.o and consensus.o respectively without making those modules dependent on CCoinsModifier nor CCoinsViewCache, we need an interface that doesn’t implement the method CCoins* AccessCoins(const uint256 &txid). This interface doesn’t need several other methods from CCoinsViewCache. You can see some code using this change at #5697
2. laanwj added the label Improvement on Feb 19, 2015
3. 
   theuni commented at 7:08 pm on February 19, 2015: member

   As discussed on IRC. The first commit (making CCoinsView an interface class and creating CCoinsViewDummy to reimplement the current CCoinsView) is confusing, and really doesn’t add any functionality. I’d prefer to skip that.
4. 
   jtimon commented at 3:14 am on February 20, 2015: contributor

   I assume you’re talking about HaveInputs(), GetValueIn(), GetPriority() and GetOutputFor() Do we want those to be virtual? Abstract classes often have methods implemented that use pure virtual methods that the child classes must implement like in here. Are we expecting them to be reimplemented by child classes?
5. 
   theuni commented at 3:18 am on February 20, 2015: member

   No, I just meant that the classes with virtual functions need virtual destructors.
6. jtimon force-pushed on Feb 20, 2015
7. 
   jtimon commented at 3:28 am on February 20, 2015: contributor

   Nits resolved.
8. 
   sipa commented at 8:21 am on February 20, 2015: member

   I would prefer moving GetOutputFor, GetPriority, GetValueIn, and HaveInputs to become validation/policy related utiliy functions in main, avoiding the issue altogether.
9. 
   jtimon commented at 7:35 pm on February 20, 2015: contributor

   haveinputs is used in consensus (see https://github.com/jtimon/bitcoin/commit/0ac0610bf054aadc621201f79432e7559aea2cf2). GetValueIn (which uses GetOutputFor) could be modified to avoid some code duplication with consensus. GetPriority could probably be moved to policy, I’m just not there yet. In any case, this change doesn’t prevent future changes in that direction. To clarify, I misunderstood theuni in his previous comment, he was just talking about the virtual destructors which I fixed, so I’m not sure what the issue you want to avoid altogether is…
10. 
    sipa commented at 11:21 am on March 1, 2015: member

    Ok, that can happen independently if it doesn’t solve any controversy. I just don’t think GetOutputFor, GetPriority, GetValueIn and HaveInputs belong in CCoins (despite me being the one who put them there).
11. 
    sipa commented at 11:46 am on March 1, 2015: member

    untested ACK, but verified b627d9abbe25a11ffca02ef1f3ab312575e5e450 as moveonly.
12. 
    theuni commented at 8:45 pm on March 15, 2015: member

    utACK
13. jtimon force-pushed on Mar 25, 2015
14. jtimon force-pushed on Mar 25, 2015
15. 
    jtimon commented at 11:17 am on March 25, 2015: contributor

    Updated doing the includes better (not inclding coins.h nor coinscache.h from main.h), after doing that it needed rebase, so I rebased it. The non-includes code remains the same.
16. jtimon force-pushed on Mar 25, 2015
17. Coins: Refactor: Separate CCoinsViewEfficient abstract class from CCoinsViewCache (make parent its class) c3b1181020
18. Coins: MOVEONLY: Move CCoinsViewCache and CCoinsModifier to coinscache.o 613b062a25
19. jtimon force-pushed on Mar 26, 2015
20. 
    jtimon commented at 12:09 pm on March 26, 2015: contributor

    Needed rebase.
21. 
    jtimon commented at 6:55 pm on April 6, 2015: contributor

    I’m having second thoughts about this, closing for now.
22. jtimon closed this on Apr 6, 2015

    ---

23. MarcoFalke locked this on Sep 8, 2021

Contributors
jtimon theuni sipa

Labels
Refactoring