WIP: Policy: Separate standard and testing policies #5180

jtimon · 2014-10-30T22:33:55Z

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.

jtimon · 2014-10-30T22:49:44Z

Since I touch chainparams, my version needed rebase...

luke-jr · 2014-10-30T23:33:49Z

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).

jtimon · 2014-10-31T20:50:53Z

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.

jtimon · 2014-12-30T01:30:04Z

Updated initial description.

jtimon · 2015-01-03T15:30:43Z

Updated code and description again without IsDust() this time.

jtimon · 2015-01-07T23:03:13Z

Rebased after #5521 has been merged for easier review.

jtimon · 2015-01-13T00:20:56Z

Closing in favor of #5652

jtimon · 2015-06-08T03:24:58Z

Updated after #6068 has been updated. Now the addition of the -policy option is left for the end as an optinal "squasheable" commit.

jtimon · 2015-06-25T19:31:56Z

Closing until some version of #6068 is merged.

jtimon · 2015-06-26T16:47:06Z

Updated opening description after rebasing and re-closing.

luke-jr · 2015-07-07T18:43:56Z

src/init.cpp

-    if (Params().RequireStandard() && !fRequireStandard)
-        return InitError(strprintf("acceptnonstdtxn is not currently supported for %s chain", chainparams.NetworkIDString()));
+    if (GetBoolArg("-acceptnonstdtxn", false))
+        return InitError(strprintf("acceptnonstdtxn is deprecated, use policy=test instead.", chainparams.NetworkIDString()));


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.

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.

…y class implementing it Rename 3 functions into CPolicy methods: - IsStandard -> policy.ApproveScript - IsStandardTx -> policy.ApproveTx - AreInputsStandard -> policy.ApproveTxInputs

…andardPolicy attributes

…o select them

…tly 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()

jtimon · 2015-07-08T12:34:38Z

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.

jtimon · 2015-07-11T12:12:30Z

I only reopened to show and discuss the updated version. But, again, closing until there's a CPolicy interface class.

jtimon force-pushed the nodepolicy2 branch from 26fcf7d to b7ac913 Compare October 30, 2014 22:48

jtimon force-pushed the nodepolicy2 branch 3 times, most recently from 1e64382 to f1b49db Compare October 31, 2014 20:18

jtimon mentioned this pull request Dec 21, 2014

Reject non-final txs even in testnet/regtest #5521

Merged

jtimon force-pushed the nodepolicy2 branch from 168edbd to 528a7b3 Compare December 30, 2014 00:51

jtimon mentioned this pull request Dec 30, 2014

Introduce CNodePolicy for putting isolated node policy code and parameters on #5071

Closed

jtimon force-pushed the nodepolicy2 branch from 528a7b3 to 41d09a6 Compare January 3, 2015 15:22

jtimon changed the title ~~Separate standard and testing policies~~ Policy: Separate standard and testing policies Jan 3, 2015

This was referenced Jan 3, 2015

Policy: Move CTxOut::IsDust() to policy.o #5114

Closed

Policy: Create CPolicy interface and CStandardPolicy class implementing it #5595

Closed

jtimon force-pushed the nodepolicy2 branch 5 times, most recently from 5cbc611 to ef91b0c Compare January 7, 2015 22:38

jtimon mentioned this pull request Jan 8, 2015

Encapsulate policy #4943

Closed

jtimon force-pushed the nodepolicy2 branch from ef91b0c to 58a219f Compare January 10, 2015 18:16

jtimon mentioned this pull request Jan 13, 2015

Policy: Continue policy movements #5652

Closed

jtimon force-pushed the nodepolicy2 branch from 58a219f to db7c615 Compare January 13, 2015 00:20

jtimon mentioned this pull request Jan 20, 2015

Separate CValidationState from main #5669

Merged

jtimon closed this Jan 21, 2015

jtimon reopened this May 13, 2015

jtimon force-pushed the nodepolicy2 branch 3 times, most recently from b26ea47 to 5f97028 Compare June 17, 2015 22:11

jtimon force-pushed the nodepolicy2 branch 3 times, most recently from 3deba59 to 3eaa099 Compare June 25, 2015 11:39

jtimon closed this Jun 25, 2015

jtimon reopened this Jun 26, 2015

jtimon force-pushed the nodepolicy2 branch from 3eaa099 to 689620a Compare June 26, 2015 16:08

jtimon closed this Jun 26, 2015

jtimon mentioned this pull request Jun 29, 2015

Scheduled full-RBF deployment #6352

Closed

jtimon mentioned this pull request Jul 6, 2015

acceptnonstdtxn option to skip (most) "non-standard transaction" checks, for testnet/regtest only #6329

Merged

jtimon reopened this Jul 6, 2015

jtimon force-pushed the nodepolicy2 branch from 689620a to 3767026 Compare July 6, 2015 22:31

luke-jr reviewed Jul 7, 2015
View reviewed changes

jtimon changed the title ~~DEPENDENT: Policy: Separate standard and testing policies~~ WIP: Policy: Separate standard and testing policies Jul 7, 2015

jtimon added 6 commits July 8, 2015 13:11

Policy: RENAME: Introduce CPolicy interface and hidden CStandardPolic…

b3493b9

…y class implementing it Rename 3 functions into CPolicy methods: - IsStandard -> policy.ApproveScript - IsStandardTx -> policy.ApproveTx - AreInputsStandard -> policy.ApproveTxInputs

Policy: Turn globals fIsBareMultisigStd and fRequireStandard into CSt…

cece42e

…andardPolicy attributes

fixup! the whole point of exposing policy options is to allow users t…

2d71478

…o select them

Introduce Container template

38d64c9

Policy: Allow selection of different policies using a factory

3ca225f

jtimon force-pushed the nodepolicy2 branch from 3767026 to e6ae422 Compare July 8, 2015 12:29

jtimon closed this Jul 11, 2015

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

WIP: Policy: Separate standard and testing policies #5180

WIP: Policy: Separate standard and testing policies #5180

Uh oh!

jtimon commented Oct 30, 2014

Uh oh!

jtimon commented Oct 30, 2014

Uh oh!

luke-jr commented Oct 30, 2014

Uh oh!

jtimon commented Oct 31, 2014

Uh oh!

jtimon commented Dec 30, 2014

Uh oh!

jtimon commented Jan 3, 2015

Uh oh!

jtimon commented Jan 7, 2015

Uh oh!

jtimon commented Jan 13, 2015

Uh oh!

jtimon commented Jun 8, 2015

Uh oh!

jtimon commented Jun 25, 2015

Uh oh!

jtimon commented Jun 26, 2015

Uh oh!

luke-jr Jul 7, 2015

Uh oh!

jtimon Jul 7, 2015

Uh oh!

jtimon commented Jul 8, 2015

Uh oh!

jtimon commented Jul 11, 2015

Uh oh!

Uh oh!

WIP: Policy: Separate standard and testing policies #5180

WIP: Policy: Separate standard and testing policies #5180

Uh oh!

Conversation

jtimon commented Oct 30, 2014

Uh oh!

jtimon commented Oct 30, 2014

Uh oh!

luke-jr commented Oct 30, 2014

Uh oh!

jtimon commented Oct 31, 2014

Uh oh!

jtimon commented Dec 30, 2014

Uh oh!

jtimon commented Jan 3, 2015

Uh oh!

jtimon commented Jan 7, 2015

Uh oh!

jtimon commented Jan 13, 2015

Uh oh!

jtimon commented Jun 8, 2015

Uh oh!

jtimon commented Jun 25, 2015

Uh oh!

jtimon commented Jun 26, 2015

Uh oh!

luke-jr Jul 7, 2015

Choose a reason for hiding this comment

Uh oh!

jtimon Jul 7, 2015

Choose a reason for hiding this comment

Uh oh!

jtimon commented Jul 8, 2015

Uh oh!

jtimon commented Jul 11, 2015

Uh oh!

Uh oh!