[Refactor] Combine scriptPubKey and amount as CTxOut in CScriptCheck #10953

pull jl2012 wants to merge 2 commits into bitcoin:master from jl2012:combine_script_amount changing 4 files +10 −17
  1. jl2012 commented at 9:40 AM on July 30, 2017: contributor

    This simplifies CScriptCheck by combining scriptPubKey and amount

  2. sipa commented at 9:44 AM on July 30, 2017: member

    Concept ACK

  3. fanquake added the label Validation on Jul 30, 2017
  4. NicolasDorier commented at 1:35 PM on July 30, 2017: contributor

    VerifyScript shouldn't have TxOut as well instead of 2 different params ?

  5. TheBlueMatt commented at 2:22 PM on July 30, 2017: member

    Concept ACK

  6. jl2012 commented at 3:12 PM on July 30, 2017: contributor

    @NicolasDorier seems can't do that because scriptPubKey and nValue go to different places.

  7. Christewart commented at 7:02 PM on July 30, 2017: member

    utack 3457ef1

  8. sipa commented at 12:05 AM on July 31, 2017: member

    @NicolasDorier VerifyScript knows nothing about transaction structure.

  9. NicolasDorier commented at 12:20 AM on July 31, 2017: contributor

    woops indeed I missed it utACK 3457ef1f19caf02c28d45545a8673ebdc4cf3997

  10. in src/validation.h:368 in 3457ef1f19 outdated 
     362 | @@ -364,17 +363,15 @@ class CScriptCheck
 363 |      PrecomputedTransactionData *txdata;
 364 |  
 365 |  public:
 366 | -    CScriptCheck(): amount(0), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
 367 | -    CScriptCheck(const CScript& scriptPubKeyIn, const CAmount amountIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) :
 368 | -        scriptPubKey(scriptPubKeyIn), amount(amountIn),
 369 | -        ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), txdata(txdataIn) { }
 370 | +    CScriptCheck(): out(CTxOut()), ptxTo(0), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
 371 | +    CScriptCheck(const CTxOut& outIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) :
    promag commented at 12:26 AM on July 31, 2017:

    Remove CTxOut(). Nit, class members are prefixed.

  11. afk11 commented at 8:26 AM on August 6, 2017: contributor

    Concept ACK

  12. [Refactor] Combine scriptPubKey and amount as CTxOut in CScriptCheck e912118786
  13. jl2012 force-pushed on Aug 30, 2017
  14. jl2012 commented at 12:39 PM on August 30, 2017: contributor

    rebased

  15. promag commented at 10:49 PM on August 30, 2017: member

    utACK e912118.

  16. theuni commented at 3:59 PM on August 31, 2017: member

    utACK e912118786f867d6821e2c1a2e4e1d4937fefd85

  17. jtimon commented at 12:47 AM on September 6, 2017: contributor

    utACK e912118786f867d6821e2c1a2e4e1d4937fefd85

  18. in src/validation.h:358 in e912118786 outdated 
     354 | @@ -355,8 +355,7 @@ bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = null
 355 |  class CScriptCheck
 356 |  {
 357 |  private:
 358 | -    CScript scriptPubKey;
 359 | -    CAmount amount;
 360 | +    CTxOut out;
    jnewbery commented at 3:30 PM on September 18, 2017:

    nit: I know it's tedious, but can you name this according to the new style guide. In this case m_tx_out

    jl2012 commented at 5:48 PM on September 19, 2017:

    Where could I see the new style guide? And what does the "m" mean? Actually I think "prevout" is the most accurate name but the name is already used.

    jnewbery commented at 5:51 PM on September 19, 2017:

    https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md

    Class member variables have a m_ prefix.

    jl2012 commented at 5:53 PM on September 19, 2017:

    actually the name out is taken from https://github.com/bitcoin/bitcoin/blob/v0.15.0.1/src/coins.h#L33 . So for consistency we have to keep it.

    jnewbery commented at 6:01 PM on September 19, 2017:

    I don't understand. That's for Coin class member. Why does the CScriptCheck member need to be named the same simply because it's of the same type?

  19. in src/validation.h:368 in e912118786 outdated 
     363 | @@ -365,17 +364,15 @@ class CScriptCheck
 364 |      PrecomputedTransactionData *txdata;
 365 |  
 366 |  public:
 367 | -    CScriptCheck(): amount(0), ptxTo(nullptr), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
 368 | -    CScriptCheck(const CScript& scriptPubKeyIn, const CAmount amountIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) :
 369 | -        scriptPubKey(scriptPubKeyIn), amount(amountIn),
 370 | -        ptxTo(&txToIn), nIn(nInIn), nFlags(nFlagsIn), cacheStore(cacheIn), error(SCRIPT_ERR_UNKNOWN_ERROR), txdata(txdataIn) { }
 371 | +    CScriptCheck(): ptxTo(nullptr), nIn(0), nFlags(0), cacheStore(false), error(SCRIPT_ERR_UNKNOWN_ERROR) {}
 372 | +    CScriptCheck(const CTxOut& outIn, const CTransaction& txToIn, unsigned int nInIn, unsigned int nFlagsIn, bool cacheIn, PrecomputedTransactionData* txdataIn) :
    jnewbery commented at 3:32 PM on September 18, 2017:

    Style guide says that arguments should be snake case, but I think it's fine name this outIn to match the style of the other arguments in the constructor.

  20. jnewbery commented at 3:32 PM on September 18, 2017: member

    utACK

  21. TheBlueMatt commented at 1:27 AM on September 21, 2017: member

    utACK e912118786f867d6821e2c1a2e4e1d4937fefd85 (modulo it would be nice to use m_out or m_tx_out as @jnewbery suggested in CScriptCheck).

  22. Rename out to m_tx_out in CScriptCheck 3a131b7244
  23. sipa merged this on Sep 22, 2017
  24. sipa closed this on Sep 22, 2017
  25. sipa referenced this in commit aeed345c9b on Sep 22, 2017
  26. underdarkskies referenced this in commit ef9bc49474 on Jul 3, 2018
  27. underdarkskies referenced this in commit bc382ff2dd on Jul 6, 2018
  28. underdarkskies referenced this in commit 10091d2356 on Jul 7, 2018
  29. underdarkskies referenced this in commit 6d0e00854a on Jul 10, 2018
  30. underdarkskies referenced this in commit 432a95fd7c on Jul 14, 2018
  31. underdarkskies referenced this in commit 1dc494d3ae on Jul 14, 2018
  32. underdarkskies referenced this in commit 2f109c28d5 on Jul 14, 2018
  33. underdarkskies referenced this in commit 900b89fff9 on Jul 14, 2018
  34. underdarkskies referenced this in commit e59032fb31 on Jul 18, 2018
  35. PastaPastaPasta referenced this in commit e67bae2794 on Jan 17, 2020
  36. PastaPastaPasta referenced this in commit b22c19fd76 on Jan 22, 2020
  37. PastaPastaPasta referenced this in commit 24d5f0b4e4 on Jan 22, 2020
  38. PastaPastaPasta referenced this in commit 1eb556fad5 on Jan 29, 2020
  39. PastaPastaPasta referenced this in commit 1b62397b3f on Jan 29, 2020
  40. PastaPastaPasta referenced this in commit f25f00aac1 on Jan 31, 2020
  41. jasonbcox referenced this in commit fddb00fc8c on Jul 15, 2020
  42. ckti referenced this in commit cda2867c5a on Mar 28, 2021
  43. furszy referenced this in commit 2f0d2d0b3a on May 26, 2021
  44. MarcoFalke locked this on Sep 8, 2021
Contributors
jl2012sipaNicolasDorierTheBlueMattChristewartpromagafk11theunijtimonjnewbery
Labels
Validation
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-30 21:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me