Factor out CCoinsView based AreInputsStandard/IsWitnessStandard #10517

pull jonasschnelli wants to merge 1 commits into bitcoin:master from jonasschnelli:2017/06/policy_compile changing 13 files +162 −115

jonasschnelli commented at 7:48 AM on June 3, 2017: contributor

It's move-onlyish. This will allow IsStandard checks without depending on the mempool, UTXO set.

This may be useful for libbitcoinconsensus and is certainly required for compiling a wallet-tool (next to some other things that are required).
jonasschnelli force-pushed on Jun 3, 2017
jonasschnelli force-pushed on Jun 3, 2017
in src/policy/inputs.h:13 in bb6f0981b7 outdated
```
   8 | +#include <string>
   9 | +
  10 | +class CCoinsViewCache;
  11 | +class CTransaction;
  12 | +
  13 | +    /**
```
benma commented at 12:19 PM on June 3, 2017:

I know it's just a move, but maybe you could fix the indentation here and below?
in src/policy/inputs.cpp:7 in bb6f0981b7 outdated
```
   0 | @@ -0,0 +1,101 @@
   1 | +// Copyright (c) 2017 The Bitcoin Core developers
   2 | +// Distributed under the MIT software license, see the accompanying
   3 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 | +
   5 | +// NOTE: This file is intended to be customised by the end user, and includes only local node policy logic
   6 | +
   7 | +#include "policy/policy.h"
```
benma commented at 12:21 PM on June 3, 2017:

I think it would be beneficial to complete the split, i.e.:

include script/standard.h instead

move MAX_P2SH_SIGOPS, MAX_STANDARD_P2WSH_STACK_ITEMS, MAX_STANDARD_P2WSH_STACK_ITEM_SIZE and MAX_STANDARD_P2WSH_SCRIPT_SIZE from policy.h to inputs.h.
benma commented at 7:57 AM on June 5, 2017:

@jonasschnelli You can include script/standard.h here instead of policy/policy.h to cut all ties :)

Mabye move the new include further down next to script/interpreter.h.
benma changes_requested
fanquake added the label Refactoring on Jun 4, 2017

Factor out CCoinsView based AreInputsStandard/IsWitnessStandard

This will allow IsStandard checks without depending on the mempool, UTXO set

2fc36f0d92

jonasschnelli force-pushed on Jun 5, 2017
jonasschnelli commented at 7:19 AM on June 5, 2017: contributor
Followed @benma's advice (force push amend commit):
- moved the input.cpp relevant constants to from policy.h to input.h
- fixed indentation
I kept the name policy.*. Renaming it to standard.* makes little sense as long as we have tx size in there as well. Can split later.
in src/Makefile.am:330 in 2fc36f0d92
```
 326 |    $(BITCOIN_CORE_H)
 327 |  
 328 | +# policy: shared between all executables that require policy rules.
 329 | +libbitcoin_policy_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES)
 330 | +libbitcoin_policy_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
 331 | +libbitcoin_policy_a_SOURCES = \
```
benma commented at 7:54 AM on June 5, 2017:

This is a bit confusing to me.

The sources list doesn't contain all of policy/*.cpp (fees.cpp and inputs.cpp could be in there?), and the ones listed below are not used together anywhere (i.e. feerate.cpp is used in libbitcoin_common which doesn't need policy.cpp, and libbitcoin_server relies on policy.cpp but not feerate.cpp).

Why is this shared library needed, and why this particular combination?

jonasschnelli commented at 8:01 AM on June 5, 2017:

It's an in-between step and allows to grow the independent policy code/module. libbitcoin_policy_a does only contain the complete independent policy classes. But we could also completely avoid the new "package" (aka library). Thoughts @theuni?
benma changes_requested
benma commented at 7:57 AM on June 5, 2017: none

(see individual comments)
ryanofsky commented at 4:43 PM on June 22, 2017: member

utACK 2fc36f0d929b6fbcefcae8d986efe38a4adbeec3

I don't know all the reasoning behind libconsensus code organization, but I can see how this would be useful for a wallet tool, and confirmed the change is move only.
jonasschnelli commented at 3:36 AM on October 5, 2017: contributor

Closing for now... I'll maybe pickup later.
jonasschnelli closed this on Oct 5, 2017
DrahtBot locked this on Sep 8, 2021

Contributors

Labels