diff --git a/src/policy/policy.h b/src/policy/policy.h
--- a/src/policy/policy.h
+++ b/src/policy/policy.h
@@ -86,7 +86,8 @@
SCRIPT_VERIFY_NULLDUMMY | SCRIPT_VERIFY_SIGPUSHONLY |
SCRIPT_VERIFY_MINIMALDATA | SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS |
SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY |
- SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | SCRIPT_VERIFY_CHECKDATASIG_SIGOPS;
+ SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | SCRIPT_VERIFY_CHECKDATASIG_SIGOPS |
+ SCRIPT_DISALLOW_SEGWIT_RECOVERY;
/**
* For convenience, standard but not mandatory verify flags.
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1635,9 +1635,10 @@
CScript pubKey2(pubKeySerialized.begin(), pubKeySerialized.end());
popstack(stack);
- // Bail out early if ALLOW_SEGWIT_RECOVERY is set, the redeem script is
- // a p2sh segwit program and it was the only item pushed into the stack
- if ((flags & SCRIPT_ALLOW_SEGWIT_RECOVERY) != 0 && stack.empty() &&
+ // Bail out early if SCRIPT_DISALLOW_SEGWIT_RECOVERY is not set, the
+ // redeem script is a p2sh segwit program, and it was the only item
+ // pushed onto the stack.
+ if ((flags & SCRIPT_DISALLOW_SEGWIT_RECOVERY) == 0 && stack.empty() &&
pubKey2.IsWitnessProgram()) {
return set_success(serror);
}
diff --git a/src/script/script_flags.h b/src/script/script_flags.h
--- a/src/script/script_flags.h
+++ b/src/script/script_flags.h
@@ -12,6 +12,7 @@
SCRIPT_VERIFY_NONE = 0,
// Evaluate P2SH subscripts (softfork safe, BIP16).
+ // Note: The Segwit Recovery feature is an exception to P2SH
SCRIPT_VERIFY_P2SH = (1U << 0),
// Passing a non-strict-DER signature or one with undefined hashtype to a
@@ -63,6 +64,7 @@
// be true".
// (softfork safe, BIP62 rule 6)
// Note: CLEANSTACK should never be used without P2SH or WITNESS.
+ // Note: The Segwit Recovery feature is an exception to CLEANSTACK
SCRIPT_VERIFY_CLEANSTACK = (1U << 8),
// Verify CHECKLOCKTIMEVERIFY
@@ -106,8 +108,9 @@
//
SCRIPT_ENABLE_SCHNORR = (1U << 19),
- // Allows the recovery of coins sent to p2sh segwit addresses
- SCRIPT_ALLOW_SEGWIT_RECOVERY = (1U << 20),
+ // The exception to CLEANSTACK and P2SH for the recovery of coins sent
+ // to p2sh segwit addresses is not allowed.
+ SCRIPT_DISALLOW_SEGWIT_RECOVERY = (1U << 20),
};
#endif // BITCOIN_SCRIPT_SCRIPTFLAGS_H
diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -29,7 +29,7 @@
SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | SCRIPT_VERIFY_MINIMALIF |
SCRIPT_VERIFY_NULLFAIL | SCRIPT_VERIFY_COMPRESSED_PUBKEYTYPE |
SCRIPT_ENABLE_SIGHASH_FORKID | SCRIPT_ENABLE_REPLAY_PROTECTION |
- SCRIPT_VERIFY_CHECKDATASIG_SIGOPS | SCRIPT_ALLOW_SEGWIT_RECOVERY;
+ SCRIPT_VERIFY_CHECKDATASIG_SIGOPS | SCRIPT_DISALLOW_SEGWIT_RECOVERY;
/**
* Valid signature cache, to avoid doing expensive ECDSA signature checking
diff --git a/src/test/data/script_tests.json b/src/test/data/script_tests.json
--- a/src/test/data/script_tests.json
+++ b/src/test/data/script_tests.json
@@ -3262,149 +3262,163 @@
[
"0x16 0x001491b24bf9f5288532960ac687abb035127b1d28a5",
"HASH160 0x14 0x17743beb429c55c942d2ec703b98c4d57c2df5c6 EQUAL",
- "CLEANSTACK,P2SH",
+ "CLEANSTACK,DISALLOW_SEGWIT_RECOVERY,P2SH",
"CLEANSTACK",
- "v0 P2SH-P2WPKH but no SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "v0 P2SH-P2WPKH with SCRIPT_DISALLOW_SEGWIT_RECOVERY"
],
[
"0x16 0x001491b24bf9f5288532960ac687abb035127b1d28a5",
"HASH160 0x14 0x17743beb429c55c942d2ec703b98c4d57c2df5c6 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"OK",
- "v0 P2SH-P2WPKH with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Valid Segwit Recovery with v0 P2SH-P2WPKH"
],
[
"0 0x16 0x001491b24bf9f5288532960ac687abb035127b1d28a5",
"HASH160 0x14 0x17743beb429c55c942d2ec703b98c4d57c2df5c6 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "v0 P2SH-P2WPKH with extra stack item and SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "v0 P2SH-P2WPKH Segwit Recovery with extra stack item"
],
[
"0x22 0x00205a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
"HASH160 0x14 0x17a6be2f8fe8e94f033e53d17beefda0f3ac4409 EQUAL",
- "CLEANSTACK,P2SH",
+ "CLEANSTACK,DISALLOW_SEGWIT_RECOVERY,P2SH",
"CLEANSTACK",
- "v0 P2SH-P2WSH but no SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "v0 P2SH-P2WSH with SCRIPT_DISALLOW_SEGWIT_RECOVERY"
],
[
"0x22 0x00205a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
"HASH160 0x14 0x17a6be2f8fe8e94f033e53d17beefda0f3ac4409 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"OK",
- "v0 P2SH-P2WSH with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Valid Segwit Recovery with v0 P2SH-P2WSH"
],
[
"0 0x22 0x00205a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
"HASH160 0x14 0x17a6be2f8fe8e94f033e53d17beefda0f3ac4409 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "v0 P2SH-P2WSH with extra stack item and SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "v0 P2SH-P2WSH Segwit Recovery with extra stack item"
],
[
"0x03 0x00015a",
"HASH160 0x14 0x40b6941895022d458de8f4bbfe27f3aaa4fb9a74 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "Invalid witness program (too short) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with invalid witness program (too short)"
],
[
"0x04 0x00025a01",
"HASH160 0x14 0x86123d8e050333a605e434ecf73128d83815b36f EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"OK",
- "Valid witness program (min allowed length) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with valid witness program (min allowed length)"
],
[
"0x2a 0x00285a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f2021222324252627",
"HASH160 0x14 0xdf7b93f88e83471b479fb219ae90e5b633d6b750 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"OK",
- "Valid witness program (max allowed length) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with valid witness program (max allowed length)"
],
[
"0x2b 0x00295a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f202122232425262728",
"HASH160 0x14 0x13aa4fcfd630508e0794dca320cac172c5790aea EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "Invalid witness program (too long) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with invalid witness program (too long)"
],
[
"0x22 0x60205a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
"HASH160 0x14 0x9b0c7017004d3818b7c833ddb3cb5547a22034d0 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"OK",
- "Valid witness program (max allowed version) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with valid witness program (max allowed version)"
],
[
"0x22 0x4f205a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
"HASH160 0x14 0x97aa1e96e49ca6d744d7344f649dd9f94bcc35eb EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "Invalid witness program (invalid version -1) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with invalid witness program (invalid version -1)"
],
[
"0x23 0x0111205a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
"HASH160 0x14 0x4b5321beb1c09f593ff3c02be4af21c7f949e101 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "Invalid witness program (invalid version 17) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with invalid witness program (invalid version 17)"
],
[
"0x23 0x00205a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f51",
"HASH160 0x14 0x8eb812176c9e71732584123dd06d3246e659b199 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "Invalid witness program (more than 2 stack items) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with invalid witness program (more than 2 stack items)"
],
[
"0x04 0x00020000",
"HASH160 0x14 0x0e01bcfe7c6f3fd2fd8f81092299369744684733 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"OK",
"Valid segwit recovery, in spite of false value being left on stack (0)"
],
[
"0x04 0x00020080",
"HASH160 0x14 0x10ddc638cb26615f867dad80efacced9e73766bc EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"OK",
"Valid segwit recovery, in spite of false value being left on stack (minus 0)"
],
+[
+ "0x04 0x00020000",
+ "HASH160 0x14 0x0e01bcfe7c6f3fd2fd8f81092299369744684733 EQUAL",
+ "CLEANSTACK,DISALLOW_SEGWIT_RECOVERY,P2SH",
+ "EVAL_FALSE",
+ "Otherwise valid segwit recovery, in spite of false value being left on stack (0), but with SCRIPT_DISALLOW_SEGWIT_RECOVERY"
+],
+[
+ "0x04 0x00020080",
+ "HASH160 0x14 0x10ddc638cb26615f867dad80efacced9e73766bc EQUAL",
+ "CLEANSTACK,DISALLOW_SEGWIT_RECOVERY,P2SH",
+ "EVAL_FALSE",
+ "Otherwise valid segwit recovery, in spite of false value being left on stack (minus 0), but with SCRIPT_DISALLOW_SEGWIT_RECOVERY"
+],
[
"0x22 0x50205a0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f",
"HASH160 0x14 0xbe02794ceede051da41b420e88a86fff2802af06 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"BAD_OPCODE",
- "Invalid witness program (OP_RESERVED in version field) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with invalid witness program (OP_RESERVED in version field)"
],
[
"0x17 0x01001491b24bf9f5288532960ac687abb035127b1d28a5",
"HASH160 0x14 0x0718743e67c1ef4911e0421f206c5ff81755718e EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "Invalid witness program (non-minimal push in version field) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with invalid witness program (non-minimal push in version field)"
],
[
"0x05 0x004c0245aa",
"HASH160 0x14 0xd3ec673296c7fd7e1a9e53bfc36f414de303e905 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "Invalid witness program (non-minimal push in program field) with SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "Segwit Recovery with invalid witness program (non-minimal push in program field)"
],
[
"0x16 0x001491b24bf9f5288532960ac687abb035127b1d28a5",
"HASH160 0x14 0x17a6be2f8fe8e94f033e53d17beefda0f3ac4409 EQUAL",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"EVAL_FALSE",
- "v0 P2SH-P2WPKH whose redeem script hash does not match P2SH output and SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "v0 P2SH-P2WPKH Segwit Recovery whose redeem script hash does not match P2SH output"
],
[
"0x16 0x001491b24bf9f5288532960ac687abb035127b1d28a5",
"1",
- "ALLOW_SEGWIT_RECOVERY,CLEANSTACK,P2SH",
+ "CLEANSTACK,P2SH",
"CLEANSTACK",
- "v0 P2SH-P2WPKH spending a non-P2SH output and SCRIPT_ALLOW_SEGWIT_RECOVERY"
+ "v0 P2SH-P2WPKH Segwit Recovery spending a non-P2SH output"
],
["CHECKSEQUENCEVERIFY tests"],
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -2080,25 +2080,25 @@
.EditPush(64, "01", "41")
.ScriptError(SCRIPT_ERR_EVAL_FALSE));
- // Tests SCRIPT_ALLOW_SEGWIT_RECOVERY
- const uint32_t allowSegwitRecoveryFlags = SCRIPT_VERIFY_CLEANSTACK |
- SCRIPT_VERIFY_P2SH |
- SCRIPT_ALLOW_SEGWIT_RECOVERY;
+ // Tests Segwit Recovery transactions and SCRIPT_DISALLOW_SEGWIT_RECOVERY
+ const uint32_t allowSegwitRecoveryFlags =
+ SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH;
tests.push_back(
TestBuilder(CScript() << OP_0 << ToByteVector(keys.pubkey0.GetID()),
- "v0 P2SH-P2WPKH but no SCRIPT_ALLOW_SEGWIT_RECOVERY",
- SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH, true)
+ "v0 P2SH-P2WPKH with SCRIPT_DISALLOW_SEGWIT_RECOVERY",
+ SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH |
+ SCRIPT_DISALLOW_SEGWIT_RECOVERY,
+ true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_CLEANSTACK));
tests.push_back(
TestBuilder(CScript() << OP_0 << ToByteVector(keys.pubkey0.GetID()),
- "v0 P2SH-P2WPKH with SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "Valid Segwit Recovery with v0 P2SH-P2WPKH",
allowSegwitRecoveryFlags, true)
.PushRedeem());
tests.push_back(
TestBuilder(CScript() << OP_0 << ToByteVector(keys.pubkey0.GetID()),
- "v0 P2SH-P2WPKH with extra stack item and "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "v0 P2SH-P2WPKH Segwit Recovery with extra stack item",
allowSegwitRecoveryFlags, true)
.Num(0)
.PushRedeem()
@@ -2108,82 +2108,82 @@
16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31}));
tests.push_back(
TestBuilder(CScript() << OP_0 << ToByteVector(dummy256),
- "v0 P2SH-P2WSH but no SCRIPT_ALLOW_SEGWIT_RECOVERY",
- SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH, true)
+ "v0 P2SH-P2WSH with SCRIPT_DISALLOW_SEGWIT_RECOVERY",
+ SCRIPT_VERIFY_CLEANSTACK | SCRIPT_VERIFY_P2SH |
+ SCRIPT_DISALLOW_SEGWIT_RECOVERY,
+ true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_CLEANSTACK));
+ tests.push_back(TestBuilder(CScript() << OP_0 << ToByteVector(dummy256),
+ "Valid Segwit Recovery with v0 P2SH-P2WSH",
+ allowSegwitRecoveryFlags, true)
+ .PushRedeem());
tests.push_back(
TestBuilder(CScript() << OP_0 << ToByteVector(dummy256),
- "v0 P2SH-P2WSH with SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "v0 P2SH-P2WSH Segwit Recovery with extra stack item",
allowSegwitRecoveryFlags, true)
- .PushRedeem());
- tests.push_back(TestBuilder(CScript() << OP_0 << ToByteVector(dummy256),
- "v0 P2SH-P2WSH with extra stack item and "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
- allowSegwitRecoveryFlags, true)
- .Num(0)
- .PushRedeem()
- .ScriptError(SCRIPT_ERR_CLEANSTACK));
- // Tests the limits of IsWitnessProgram along with
- // SCRIPT_ALLOW_SEGWIT_RECOVERY
+ .Num(0)
+ .PushRedeem()
+ .ScriptError(SCRIPT_ERR_CLEANSTACK));
+ // Tests the limits of IsWitnessProgram without
+ // SCRIPT_DISALLOW_SEGWIT_RECOVERY.
std::vector<uint8_t> shortprogram({90, 1});
tests.push_back(
TestBuilder(CScript() << OP_0
<< std::vector<uint8_t>(shortprogram.begin(),
shortprogram.end() - 1),
- "Invalid witness program (too short) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "Segwit Recovery with invalid witness program (too short)",
allowSegwitRecoveryFlags, true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_CLEANSTACK));
tests.push_back(
- TestBuilder(CScript() << OP_0 << shortprogram,
- "Valid witness program (min allowed length) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
- allowSegwitRecoveryFlags, true)
+ TestBuilder(
+ CScript() << OP_0 << shortprogram,
+ "Segwit Recovery with valid witness program (min allowed length)",
+ allowSegwitRecoveryFlags, true)
.PushRedeem());
std::vector<uint8_t> longprogram(
{90, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13,
14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27,
28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40});
tests.push_back(
- TestBuilder(CScript() << OP_0
- << std::vector<uint8_t>(longprogram.begin(),
- longprogram.end() - 1),
- "Valid witness program (max allowed length) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
- allowSegwitRecoveryFlags, true)
+ TestBuilder(
+ CScript() << OP_0
+ << std::vector<uint8_t>(longprogram.begin(),
+ longprogram.end() - 1),
+ "Segwit Recovery with valid witness program (max allowed length)",
+ allowSegwitRecoveryFlags, true)
.PushRedeem());
- tests.push_back(TestBuilder(CScript() << OP_0 << longprogram,
- "Invalid witness program (too long) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
- allowSegwitRecoveryFlags, true)
- .PushRedeem()
- .ScriptError(SCRIPT_ERR_CLEANSTACK));
tests.push_back(
- TestBuilder(CScript() << OP_16 << ToByteVector(dummy256),
- "Valid witness program (max allowed version) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ TestBuilder(CScript() << OP_0 << longprogram,
+ "Segwit Recovery with invalid witness program (too long)",
allowSegwitRecoveryFlags, true)
+ .PushRedeem()
+ .ScriptError(SCRIPT_ERR_CLEANSTACK));
+ tests.push_back(
+ TestBuilder(
+ CScript() << OP_16 << ToByteVector(dummy256),
+ "Segwit Recovery with valid witness program (max allowed version)",
+ allowSegwitRecoveryFlags, true)
.PushRedeem());
tests.push_back(
- TestBuilder(CScript() << OP_1NEGATE << ToByteVector(dummy256),
- "Invalid witness program (invalid version -1) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
- allowSegwitRecoveryFlags, true)
+ TestBuilder(
+ CScript() << OP_1NEGATE << ToByteVector(dummy256),
+ "Segwit Recovery with invalid witness program (invalid version -1)",
+ allowSegwitRecoveryFlags, true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_CLEANSTACK));
tests.push_back(
- TestBuilder(CScript() << 17 << ToByteVector(dummy256),
- "Invalid witness program (invalid version 17) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
- allowSegwitRecoveryFlags, true)
+ TestBuilder(
+ CScript() << 17 << ToByteVector(dummy256),
+ "Segwit Recovery with invalid witness program (invalid version 17)",
+ allowSegwitRecoveryFlags, true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_CLEANSTACK));
tests.push_back(
TestBuilder(CScript() << OP_0 << ToByteVector(dummy256) << OP_1,
- "Invalid witness program (more than 2 stack "
- "items) with SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "Segwit Recovery with invalid witness program (more than 2 "
+ "stack items)",
allowSegwitRecoveryFlags, true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_CLEANSTACK));
@@ -2199,12 +2199,29 @@
"on stack (minus 0)",
allowSegwitRecoveryFlags, true)
.PushRedeem());
+ tests.push_back(
+ TestBuilder(CScript() << OP_0 << std::vector<uint8_t>({0, 0}),
+ "Otherwise valid segwit recovery, in spite of false value "
+ "being left "
+ "on stack (0), but with SCRIPT_DISALLOW_SEGWIT_RECOVERY",
+ allowSegwitRecoveryFlags | SCRIPT_DISALLOW_SEGWIT_RECOVERY,
+ true)
+ .PushRedeem()
+ .ScriptError(SCRIPT_ERR_EVAL_FALSE));
tests.push_back(
TestBuilder(
- CScript() << OP_RESERVED << ToByteVector(dummy256),
- "Invalid witness program (OP_RESERVED in version field) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
- allowSegwitRecoveryFlags, true)
+ CScript() << OP_0 << std::vector<uint8_t>({0, 0x80}),
+ "Otherwise valid segwit recovery, in spite of false value being "
+ "left "
+ "on stack (minus 0), but with SCRIPT_DISALLOW_SEGWIT_RECOVERY",
+ allowSegwitRecoveryFlags | SCRIPT_DISALLOW_SEGWIT_RECOVERY, true)
+ .PushRedeem()
+ .ScriptError(SCRIPT_ERR_EVAL_FALSE));
+ tests.push_back(
+ TestBuilder(CScript() << OP_RESERVED << ToByteVector(dummy256),
+ "Segwit Recovery with invalid witness program (OP_RESERVED "
+ "in version field)",
+ allowSegwitRecoveryFlags, true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_BAD_OPCODE));
const uint8_t nonmin_push_00[] = {1, 0};
@@ -2212,8 +2229,8 @@
TestBuilder(
CScript(&nonmin_push_00[0], &nonmin_push_00[sizeof(nonmin_push_00)])
<< ToByteVector(keys.pubkey0.GetID()),
- "Invalid witness program (non-minimal push in version field) with "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "Segwit Recovery with invalid witness program (non-minimal push in "
+ "version field)",
allowSegwitRecoveryFlags, true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_CLEANSTACK));
@@ -2222,22 +2239,22 @@
TestBuilder((CScript() << OP_0) +
CScript(&nonmin_push_45aa[0],
&nonmin_push_45aa[sizeof(nonmin_push_45aa)]),
- "Invalid witness program (non-minimal push in program "
- "field) with SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "Segwit Recovery with invalid witness program (non-minimal "
+ "push in program field)",
allowSegwitRecoveryFlags, true)
.PushRedeem()
.ScriptError(SCRIPT_ERR_CLEANSTACK));
tests.push_back(
TestBuilder(CScript() << OP_0 << ToByteVector(dummy256),
- "v0 P2SH-P2WPKH whose redeem script hash does not match "
- "P2SH output and SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "v0 P2SH-P2WPKH Segwit Recovery whose redeem script hash "
+ "does not match "
+ "P2SH output",
allowSegwitRecoveryFlags, true)
.Push(CScript() << OP_0 << ToByteVector(keys.pubkey0.GetID()))
.ScriptError(SCRIPT_ERR_EVAL_FALSE));
tests.push_back(
TestBuilder(CScript() << OP_1,
- "v0 P2SH-P2WPKH spending a non-P2SH output and "
- "SCRIPT_ALLOW_SEGWIT_RECOVERY",
+ "v0 P2SH-P2WPKH Segwit Recovery spending a non-P2SH output",
allowSegwitRecoveryFlags)
.Push(CScript() << OP_0 << ToByteVector(keys.pubkey0.GetID()))
.ScriptError(SCRIPT_ERR_CLEANSTACK));
diff --git a/src/test/scriptflags.cpp b/src/test/scriptflags.cpp
--- a/src/test/scriptflags.cpp
+++ b/src/test/scriptflags.cpp
@@ -33,7 +33,7 @@
{"REPLAY_PROTECTION", SCRIPT_ENABLE_REPLAY_PROTECTION},
{"CHECKDATASIG", SCRIPT_VERIFY_CHECKDATASIG_SIGOPS},
{"SCHNORR", SCRIPT_ENABLE_SCHNORR},
- {"ALLOW_SEGWIT_RECOVERY", SCRIPT_ALLOW_SEGWIT_RECOVERY},
+ {"DISALLOW_SEGWIT_RECOVERY", SCRIPT_DISALLOW_SEGWIT_RECOVERY},
};
uint32_t ParseScriptFlags(std::string strFlags) {
diff --git a/src/test/sigcache_tests.cpp b/src/test/sigcache_tests.cpp
--- a/src/test/sigcache_tests.cpp
+++ b/src/test/sigcache_tests.cpp
@@ -38,7 +38,7 @@
SCRIPT_VERIFY_CHECKSEQUENCEVERIFY | SCRIPT_VERIFY_MINIMALIF |
SCRIPT_VERIFY_NULLFAIL | SCRIPT_VERIFY_COMPRESSED_PUBKEYTYPE |
SCRIPT_ENABLE_SIGHASH_FORKID | SCRIPT_ENABLE_REPLAY_PROTECTION |
- SCRIPT_VERIFY_CHECKDATASIG_SIGOPS | SCRIPT_ALLOW_SEGWIT_RECOVERY;
+ SCRIPT_VERIFY_CHECKDATASIG_SIGOPS | SCRIPT_DISALLOW_SEGWIT_RECOVERY;
/* We will be testing that these flags DO affect the cache entry. The expected
* behaviour is that flags which are not explicitly listed as invariant in
* script/sigcache.cpp will affect the cache entry. Here we will thus enforce
diff --git a/src/validation.cpp b/src/validation.cpp
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -719,9 +719,6 @@
}
if (IsGreatWallEnabledForCurrentBlock(config)) {
- if (!fRequireStandard) {
- extraFlags |= SCRIPT_ALLOW_SEGWIT_RECOVERY;
- }
extraFlags |= SCRIPT_ENABLE_SCHNORR;
}
@@ -1238,14 +1235,8 @@
// This differs from MANDATORY_SCRIPT_VERIFY_FLAGS as it contains
// additional upgrade flags (see AcceptToMemoryPoolWorker variable
// extraFlags).
- // Even though it is not a mandatory flag,
- // SCRIPT_ALLOW_SEGWIT_RECOVERY is strictly more permissive than the
- // set of standard flags. It therefore needs to be added in order to
- // check if we need to penalize the peer that sent us the
- // transaction or not.
uint32_t mandatoryFlags =
- (flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS) |
- SCRIPT_ALLOW_SEGWIT_RECOVERY;
+ flags & ~STANDARD_NOT_MANDATORY_VERIFY_FLAGS;
if (flags != mandatoryFlags) {
// Check whether the failure was caused by a non-mandatory
// script verification check. If so, don't trigger DoS
@@ -1620,13 +1611,11 @@
flags |= SCRIPT_VERIFY_CLEANSTACK;
}
- // If the Great Wall fork is enabled, we start accepting transactions
- // recovering coins sent to segwit addresses. We also start accepting
+ // If the Great Wall fork is enabled, we start accepting
// 65/64-byte Schnorr signatures in CHECKSIG and CHECKDATASIG respectively,
// and their verify variants. We also stop accepting 65 byte signatures in
// CHECKMULTISIG and its verify variant.
if (IsGreatWallEnabled(config, pChainTip)) {
- flags |= SCRIPT_ALLOW_SEGWIT_RECOVERY;
flags |= SCRIPT_ENABLE_SCHNORR;
}
diff --git a/test/functional/abc-segwit-recovery-activation.py b/test/functional/abc-segwit-recovery.py
rename from test/functional/abc-segwit-recovery-activation.py
rename to test/functional/abc-segwit-recovery.py
--- a/test/functional/abc-segwit-recovery-activation.py
+++ b/test/functional/abc-segwit-recovery.py
@@ -4,15 +4,19 @@
# Distributed under the MIT software license, see the accompanying
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
"""
-This test checks activation of the SCRIPT_ALLOW_SEGWIT_RECOVERY flag
+This test checks that blocks containing segwit recovery transactions will be accepted,
+that segwit recovery transactions are rejected from mempool acceptance (even with
+-acceptnonstdtxn=1), and that segwit recovery transactions don't result in bans.
"""
+import time
+
from test_framework.blocktools import (
create_block,
create_coinbase,
make_conform_to_ctor,
)
-from test_framework.comptool import RejectResult, TestInstance, TestManager
+from test_framework.comptool import TestInstance, TestManager
from test_framework.messages import (
COIN,
COutPoint,
@@ -36,16 +40,11 @@
)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
- assert_equal,
assert_raises_rpc_error,
sync_blocks,
)
-# far into the future
-GREAT_WALL_START_TIME = 2000000000
-
-# First blocks (initial coinbases, pre-fork test blocks) happen 1 day before.
-FIRST_BLOCK_TIME = GREAT_WALL_START_TIME - 86400
+TEST_TIME = int(time.time())
# Error due to non clean stack
CLEANSTACK_ERROR = b'non-mandatory-script-verify-flag (Script did not clean its stack)'
@@ -63,7 +62,7 @@
self.n = n
-class SegwitRecoveryActivationTest(BitcoinTestFramework):
+class SegwitRecoveryTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 2
@@ -72,35 +71,28 @@
self.tip = None
self.blocks = {}
# We have 2 nodes:
- # 1) node_nonstd (nodes[0]) accepts non-standard txns. It's used to
- # test the activation itself via TestManager.
+ # 1) node_nonstd (nodes[0]) accepts non-standard txns. It does not
+ # accept Segwit recovery transactions, since it is included in
+ # standard flags, and transactions that violate these flags are
+ # never accepted into the mempool.
# 2) node_std (nodes[1]) doesn't accept non-standard txns and
# doesn't have us whitelisted. It's used to test for bans, as we
# connect directly to it via mininode and send a segwit spending
- # txn. This transaction is non-standard and, before activation,
- # also invalid. We check, before and after activation, that
- # sending this transaction doesn't result in a ban.
+ # txn. This transaction is non-standard. We check that sending
+ # this transaction doesn't result in a ban.
# Nodes are connected to each other, so node_std receives blocks and
# transactions that node_nonstd has accepted. Since we are checking
# that segwit spending txn are not resulting in bans, node_nonstd
# doesn't get banned when forwarding this kind of transactions to
# node_std.
self.extra_args = [['-whitelist=127.0.0.1',
- "-acceptnonstdtxn",
- "-greatwallactivationtime={}".format(
- GREAT_WALL_START_TIME),
- "-replayprotectionactivationtime={}".format(
- 2 * GREAT_WALL_START_TIME)],
- ["-acceptnonstdtxn=0",
- "-greatwallactivationtime={}".format(
- GREAT_WALL_START_TIME),
- "-replayprotectionactivationtime={}".format(
- 2 * GREAT_WALL_START_TIME)]]
+ "-acceptnonstdtxn"],
+ ["-acceptnonstdtxn=0"]]
def run_test(self):
# Move the mocktime up to activation
for node in self.nodes:
- node.setmocktime(GREAT_WALL_START_TIME)
+ node.setmocktime(TEST_TIME)
test = TestManager(self, self.options.tmpdir)
# TestManager only connects to node_nonstd (nodes[0])
test.add_all_connections([self.nodes[0]])
@@ -112,7 +104,7 @@
def next_block(self, number):
if self.tip == None:
base_block_hash = self.genesis_hash
- block_time = FIRST_BLOCK_TIME
+ block_time = TEST_TIME
else:
base_block_hash = self.tip.sha256
block_time = self.tip.nTime + 1
@@ -189,8 +181,6 @@
# 2) txspend: spends outputs from segwit addresses
def create_segwit_fund_and_spend_tx(spend, case0=False):
if not case0:
- # To make sure we'll be able to recover coins sent to segwit addresses,
- # we test using historical recoveries from btc.com:
# Spending from a P2SH-P2WPKH coin,
# txhash:a45698363249312f8d3d93676aa714be59b0bd758e62fa054fb1ea6218480691
redeem_script0 = bytearray.fromhex(
@@ -281,99 +271,38 @@
txfund_case0, txspend_case0 = create_segwit_fund_and_spend_tx(
out[1], True)
- # Create blocks to get closer to activate the fork.
# Mine txfund, as it can't go into node_std mempool because it's
# nonstandard.
b = block(5555)
- b.nTime = GREAT_WALL_START_TIME - 1
update_block(5555, [txfund, txfund_case0])
yield accepted()
- for i in range(5):
- block(5100 + i)
- test.blocks_and_transactions.append([self.tip, True])
- yield test
-
# Since the TestManager is not connected to node_std, we must check
# both nodes are synchronized before continuing.
sync_blocks(self.nodes)
- # Check we are just before the activation time
- assert_equal(node_nonstd.getblockheader(
- node_nonstd.getbestblockhash())['mediantime'], GREAT_WALL_START_TIME - 1)
- assert_equal(node_std.getblockheader(
- node_std.getbestblockhash())['mediantime'], GREAT_WALL_START_TIME - 1)
-
- # Before the fork, segwit spending txns are rejected.
- assert_raises_rpc_error(-26, RPC_CLEANSTACK_ERROR,
- node_nonstd.sendrawtransaction, ToHex(txspend))
- assert_raises_rpc_error(-26, RPC_CLEANSTACK_ERROR,
- node_std.sendrawtransaction, ToHex(txspend))
- assert_raises_rpc_error(-26, RPC_EVAL_FALSE_ERROR,
- node_nonstd.sendrawtransaction, ToHex(txspend_case0))
- assert_raises_rpc_error(-26, RPC_EVAL_FALSE_ERROR,
- node_std.sendrawtransaction, ToHex(txspend_case0))
-
- # Blocks containing segwit spending txns are rejected as well.
- block(2)
- update_block(2, [txspend, txspend_case0])
- yield rejected(RejectResult(16, b'blk-bad-inputs'))
-
- # Rewind bad block
- tip(5104)
-
- # Check that non-upgraded nodes checking for standardness are not
- # banning nodes sending segwit spending txns.
- check_for_no_ban_on_rejected_tx(txspend, 64, CLEANSTACK_ERROR)
- check_for_no_ban_on_rejected_tx(txspend_case0, 64, EVAL_FALSE_ERROR)
-
- # Activate the fork in both nodes!
- forkblock = block(5556)
- yield accepted()
- sync_blocks(self.nodes)
-
- # Check we just activated the fork
- assert_equal(node_nonstd.getblockheader(
- node_nonstd.getbestblockhash())['mediantime'], GREAT_WALL_START_TIME)
- assert_equal(node_std.getblockheader(
- node_std.getbestblockhash())['mediantime'], GREAT_WALL_START_TIME)
-
# Check that upgraded nodes checking for standardness are not banning
# nodes sending segwit spending txns.
check_for_no_ban_on_rejected_tx(txspend, 64, CLEANSTACK_ERROR)
check_for_no_ban_on_rejected_tx(txspend_case0, 64, EVAL_FALSE_ERROR)
- # Segwit spending txns are accepted in the mempool of nodes not checking
- # for standardness, but rejected in nodes that check.
- node_nonstd.sendrawtransaction(ToHex(txspend))
- node_nonstd.sendrawtransaction(ToHex(txspend_case0))
- check_mempool_equal(node_nonstd, [txspend, txspend_case0])
+ # Segwit recovery txns are never accepted into the mempool,
+ # as they are included in standard flags.
+ assert_raises_rpc_error(-26, RPC_CLEANSTACK_ERROR,
+ node_nonstd.sendrawtransaction, ToHex(txspend))
+ assert_raises_rpc_error(-26, RPC_EVAL_FALSE_ERROR,
+ node_nonstd.sendrawtransaction, ToHex(txspend_case0))
assert_raises_rpc_error(-26, RPC_CLEANSTACK_ERROR,
node_std.sendrawtransaction, ToHex(txspend))
assert_raises_rpc_error(-26, RPC_EVAL_FALSE_ERROR,
node_std.sendrawtransaction, ToHex(txspend_case0))
- # Blocks containing segwit spending txns are now accepted in both
- # nodes.
+ # Blocks containing segwit spending txns are accepted in both nodes.
block(5)
postforkblock = update_block(5, [txspend, txspend_case0])
yield accepted()
sync_blocks(self.nodes)
- # Ok, now we check if a reorg work properly accross the activation.
- node_nonstd.invalidateblock(postforkblock.hash)
- check_mempool_equal(node_nonstd, [txspend, txspend_case0])
-
- # Also check that nodes checking for standardness don't return a segwit
- # spending txn into the mempool when disconnecting a block.
- node_std.invalidateblock(postforkblock.hash)
- assert(len(node_std.getrawmempool()) == 0)
-
- # Deactivate the fork. The spending tx has been evicted from the
- # mempool
- node_nonstd.invalidateblock(forkblock.hash)
- assert(len(node_nonstd.getrawmempool()) == 0)
-
if __name__ == '__main__':
- SegwitRecoveryActivationTest().main()
+ SegwitRecoveryTest().main()