WIP: Policy: Separate standard and testing policies #5180

1. 
   jtimon commented at 10:33 pm on October 30, 2014: contributor

   EDIT:

   This prepares AcceptToMemoryPool to be decoupled from Params() and creates a -policy command-line option to select the standard or test policy independently of the chain mode (ie main, testnet3, regtest).

   It’s based on #6068. @petertodd already acked the concept here: #5521 (comment)

   OLD: Instead of being based on #5071 as initially, it doesn’t go as far as that PR. A rebased version of #5071 on top of this PR can be found here: https://github.com/jtimon/bitcoin/tree/nodepolicy_5180 Instead of being based on #5071 it is based on #5521 (plus one doubt-commit).

   Please let’s not delay policy.o even if it’s only with minRelayTxFee. Later proposals to expand policy.o become more readable once the first step has been walked. Even better, later proposed changes to main.o may become proposed changes to policy.o!! In my opinion the biggest development bottleneck is reviewing changes to main.o.

   /EDIT It’s based on #5071 and eliminates Params().RequireStandard() by separating two policies for the 4 networks accessible with Params() as discussed in that PR. It also includes a couple of “squashme” commits that solve my complains on #5071 in case you @luke-jr want to take them.

2. jtimon force-pushed on Oct 30, 2014
3. 
   jtimon commented at 10:49 pm on October 30, 2014: contributor

   Since I touch chainparams, my version needed rebase…
4. 
   luke-jr commented at 11:33 pm on October 30, 2014: member

   Disagree with per-network policies and global Policy() function; the former overcomplicates modifications for no real benefit, and the latter would make multiple policies more difficult to add in circumstances where they would actually be desirable (ie, within the same running instance).
5. jtimon force-pushed on Oct 31, 2014
6. jtimon force-pushed on Oct 31, 2014
7. jtimon force-pushed on Oct 31, 2014
8. 
   jtimon commented at 8:50 pm on October 31, 2014: contributor

   It’s not a per-network policy, please, read the code. It’s quite the opposite actually: it’s decoupling policies from networks. Previously the user can only select 4 {policy, network} pairs: {standard, mainnet}, {test, testnet}, {test, regtest}, {standard, unitest} With 2 separate policy classes the available combinations are 8: {standard, test} x {mainnet, testnet, regtest, unitest}. It also simplifies modifications.

   A pointer to an abstract class is by all means more flexible than a reference to a an extern object when it comes to support multiple policies. With Policy::Factory() you can make as many of them as you want, as shown in the example. If you want to avoid managing memory manually, Policy::Pool() can be used instead. Of course, its interface can be simplified to Policy::FactoryPool() (hiding CPolicyPool class). If we’re not going to have several policies with different states for the same policy type, Policy::Simple() is enough.. You can even hide CNodePolicy, exposing the abstract class is enough. In fact, what is the point in exposing the abstract class if you’re not going to use polymorphism?

   I will of course clean this up, but I hope this can convince you that CNodePolicy::fRequireStandardTx, exposing CNodePolicy and the extern CNodePolicy in main are mistakes.

   I’m indifferent to “coins: GetTxFees method” because I don’t see it as specially useful, but if the new method is not even going to be used (as in “SQUASHME: actually use GetTxFees method”) I would say NACK. It seems completely orthogonal to the rest of the PR anyway.

9. jtimon force-pushed on Dec 30, 2014
10. 
    jtimon commented at 1:30 am on December 30, 2014: contributor

    Updated initial description.
11. jtimon force-pushed on Jan 3, 2015
12. 
    jtimon commented at 3:30 pm on January 3, 2015: contributor

    Updated code and description again without IsDust() this time.
13. jtimon renamed this:
    Separate standard and testing policies
    Policy: Separate standard and testing policies
    on Jan 3, 2015
14. jtimon force-pushed on Jan 3, 2015
15. jtimon force-pushed on Jan 4, 2015
16. jtimon force-pushed on Jan 4, 2015
17. jtimon force-pushed on Jan 7, 2015
18. jtimon force-pushed on Jan 7, 2015
19. 
    jtimon commented at 11:03 pm on January 7, 2015: contributor

    Rebased after #5521 has been merged for easier review.
20. jtimon force-pushed on Jan 10, 2015
21. jtimon force-pushed on Jan 13, 2015
22. 
    jtimon commented at 0:20 am on January 13, 2015: contributor

    Closing in favor of #5652
23. jtimon closed this on Jan 21, 2015

    ---

24. jtimon reopened this on May 13, 2015

    ---

25. jtimon force-pushed on May 13, 2015
26. jtimon force-pushed on May 13, 2015
27. jtimon closed this on May 13, 2015

    ---

28. jtimon reopened this on Jun 6, 2015

    ---

29. jtimon force-pushed on Jun 6, 2015
30. jtimon force-pushed on Jun 6, 2015
31. jtimon renamed this:
    Policy: Separate standard and testing policies
    DEPENDENT: Policy: Separate standard and testing policies
    on Jun 6, 2015
32. jtimon force-pushed on Jun 6, 2015
33. jtimon force-pushed on Jun 8, 2015
34. 
    jtimon commented at 3:24 am on June 8, 2015: contributor

    Updated after #6068 has been updated. Now the addition of the -policy option is left for the end as an optinal “squasheable” commit.
35. jtimon force-pushed on Jun 11, 2015
36. jtimon force-pushed on Jun 17, 2015
37. jtimon force-pushed on Jun 17, 2015
38. jtimon force-pushed on Jun 24, 2015
39. jtimon force-pushed on Jun 24, 2015
40. jtimon force-pushed on Jun 25, 2015
41. 
    jtimon commented at 7:31 pm on June 25, 2015: contributor

    Closing until some version of #6068 is merged.
42. jtimon closed this on Jun 25, 2015

    ---

43. jtimon reopened this on Jun 26, 2015

    ---

44. jtimon force-pushed on Jun 26, 2015
45. jtimon closed this on Jun 26, 2015

    ---

46. 
    jtimon commented at 4:47 pm on June 26, 2015: contributor

    Updated opening description after rebasing and re-closing.
47. jtimon reopened this on Jul 6, 2015

    ---

48. jtimon force-pushed on Jul 6, 2015
49. in src/init.cpp: in 3767026eb1 outdated

    818@@ -820,9 +819,8 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
    819             return InitError(strprintf(_("Invalid amount for -minrelaytxfee=<amount>: '%s'"), mapArgs["-minrelaytxfee"]));
    820     }
    821 
    822-    fRequireStandard = !GetBoolArg("-acceptnonstdtxn", !Params().RequireStandard());
    823-    if (Params().RequireStandard() && !fRequireStandard)
    824-        return InitError(strprintf("acceptnonstdtxn is not currently supported for %s chain", chainparams.NetworkIDString()));
    825+    if (GetBoolArg("-acceptnonstdtxn", false))
    826+        return InitError(strprintf("acceptnonstdtxn is deprecated, use policy=test instead.", chainparams.NetworkIDString()));
    

    ---

    ---

    

    luke-jr commented at 6:43 pm on July 7, 2015:

    acceptnonstdtxn affects only one facet of the policy. It shouldn’t be deprecated unless you add a way to configure the flag some other way.

    ---

    

    jtimon commented at 6:54 pm on July 7, 2015:

    The default -acceptnonstdtxn=0 is equivalent to the default -policy=standard. To use the equivalent of -acceptnonstdtxn=1, you use -policy=test. This has the advantages of decoupling policy from Params() and that adding new policies (say, -policy=fss_rbf) becomes easier in the future. Anyway, this needs rebase.
50. jtimon renamed this:
    DEPENDENT: Policy: Separate standard and testing policies
    WIP: Policy: Separate standard and testing policies
    on Jul 7, 2015
51. Policy: RENAME: Introduce CPolicy interface and hidden CStandardPolicy class implementing it

    Rename 3 functions into CPolicy methods:
    
    - IsStandard -> policy.ApproveScript
    - IsStandardTx -> policy.ApproveTx
    - AreInputsStandard -> policy.ApproveTxInputs

    b3493b99ad
52. Policy: Turn globals fIsBareMultisigStd and fRequireStandard into CStandardPolicy attributes cece42eb90
53. fixup! the whole point of exposing policy options is to allow users to select them 2d71478a68
54. Introduce Container template 38d64c96ff
55. Policy: Allow selection of different policies using a factory 3ca225fbf2
56. Policy: Separate Standard and test policies to select them independently of the chain

    ...by replacing CChainParams::fRequireStandard with CChainParams::strDefaultPolicy
    
    Also expose it as an option (-policy=test) equivalent to -acceptnonstdtxn.
    This decouples policy/policy.cpp from Params()

    e6ae42255e
57. jtimon force-pushed on Jul 8, 2015
58. 
    jtimon commented at 12:34 pm on July 8, 2015: contributor

    Updated without deprecating anything. Now -policy overwrites the chain selection but -acceptnonstdtxn overwrites -policy (at least for policies standard and test, other policies may not even have a -acceptnonstdtxn option). I don’t care about keeping -acceptnonstdtxn even if you can just use -policy=test instead of -acceptnonstdtxn=1 (so keeping it is redundant), to me the important thing is decoupling policy from Params() and having a factory to make the creation maintenance of custom policies easier.
59. 
    jtimon commented at 12:12 pm on July 11, 2015: contributor

    I only reopened to show and discuss the updated version. But, again, closing until there’s a CPolicy interface class.
60. jtimon closed this on Jul 11, 2015

    ---

61. MarcoFalke locked this on Sep 8, 2021

Contributors
jtimon luke-jr