Changed the comments regarding the ALL_VERIFY_FLAGS.

After looking into the code, I think the workaround (removing some Flags from ALL_VERIFY_FLAGS) is not a workaround, but a valid and needed change, in order to keep the existing functionality.

Still, I consider that a deep research into the code is needed, in order to refactor all the uses of these Flags. Since these Flags affect the way the script is executed, the different invocations to the script engine should be aware of what flags are being used 8as a parameter, for example). At this moment, these Flags are hardcoded, and that shouldn,t be the case. If we want to harcode the Verification Flags used when validating the script, then we shoul
Still, I consider that a deep research into the code is needed, in order to refactor all the uses of these Flags. Since these Flags affect the way the script is executed, the different invocations to the script engine should be aware of what flags are being used 8as a parameter, for example). At this moment, these Flags are hardcoded in some places, and that shouldn't be the case (unles there is a clear consensus about that).

The current commit runs OK and the legacy tests also run OK. But we should make it clear that ALL_VERIFY_FLAGS doen NOT measn ALL of them, just the ones that allow a "safe" script execution.
parent 710c434f
......@@ -91,16 +91,22 @@ public class Script {
PUBKEYTYPE // June 26, 29018.
}
// NOTE:
// Some of the new verifications implemented in the Script are not compatible with legacy tests:
// - SIGHASH_FORKID: forces the Script to verify the ForkID flag in the SIGHASH_BYTE is enabled
// - PUBKETTYPE: forces the Script to checkh if the public key compression is OK
//
// Some legacy tests are not compilant with the previous 2 flags. The "ALL_VERIFY_FLAGS" Set
// (Which is used by those tests) includes all the Flags, so in order to keep the legacy behaviour of those tests,
// we redefine this collection so it does NOT include them. Temporary solution.
public static final EnumSet<VerifyFlag> ALL_VERIFY_FLAGS = EnumSet.complementOf(EnumSet.of(VerifyFlag.SIGHASH_FORKID, VerifyFlag.PUBKEYTYPE));
// The outcome of the script execution is affected by the Verification flags used. The more verifications are
// implemented, the more restrictions are applied on it. The ALL_VERIFY_FLAGS variable is used to store those
// verifications that can be used as a Basis for executing and validating a Script. So this Set of Flags is used
// through the Bitcoin-core and several tests when some script needs to be executed.
// After the implementation of the last verification Flags, including all of them in this Set is not safe anymore,
// since some of these flags affect the outcome of the script to a big extent, which can make other legacy tests
// to fail.
// For instance, the SIGHASH_FORKID Flag forces the Script engine to expect all the Signatures to have the SIGHASH
// FORK ID bit set. The REPLAY_PROTECTION flag, on the other hand, changes the way the Transaction Hash is
// calculated.
// A possible solution might be to parameterize all the calls to the Script Engine, adding the Flags as a new
// parameter, and refactor the whole project so every single call needs to acknowledge the Verification Flags used.
// This refactoring is a possible solution. At this moment, and in order not to break the existing code and the
// legacy tests, we remove from the SET those flags which affect the Script the most and might break the legacy code.
public static final EnumSet<VerifyFlag> ALL_VERIFY_FLAGS = EnumSet.complementOf(EnumSet.of(VerifyFlag.SIGHASH_FORKID, VerifyFlag.REPLAY_PROTECTION));
private static final Logger log = LoggerFactory.getLogger(Script.class);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment