From ec9ad5f199e4ec6eb05d8ea47016413413fe17e0 Mon Sep 17 00:00:00 2001 From: Patrick Strateman Date: Tue, 19 Apr 2016 13:07:16 -0700 Subject: [PATCH 1/4] Replace memcmp with std::equal in CScript::FindAndDelete Function is stl; std::equal just makes more sense. --- src/script/script.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/script.h b/src/script/script.h index d2a68a07b..ef3af21d6 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -574,7 +574,7 @@ public: opcodetype opcode; do { - while (end() - pc >= (long)b.size() && memcmp(&pc[0], &b[0], b.size()) == 0) + while (end() - pc >= (long)b.size() && std::equal(b.begin(), b.end(), pc)) { pc = erase(pc, pc + b.size()); ++nFound; From c0f660c3a39e3b6d75d9a6bf8a9824c347c321b8 Mon Sep 17 00:00:00 2001 From: Patrick Strateman Date: Tue, 19 Apr 2016 13:13:46 -0700 Subject: [PATCH 2/4] Replace c-style cast with c++ style static_cast. --- src/script/script.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/script/script.h b/src/script/script.h index ef3af21d6..0503b39a7 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -574,7 +574,7 @@ public: opcodetype opcode; do { - while (end() - pc >= (long)b.size() && std::equal(b.begin(), b.end(), pc)) + while (static_cast(end() - pc) >= b.size() && std::equal(b.begin(), b.end(), pc)) { pc = erase(pc, pc + b.size()); ++nFound; From e2a30bc9a9f7d2969e52632f8e8942a4e72f4ba6 Mon Sep 17 00:00:00 2001 From: Gavin Andresen Date: Wed, 3 Feb 2016 16:15:18 -0500 Subject: [PATCH 3/4] Unit test for CScript::FindAndDelete --- src/test/script_tests.cpp | 117 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index d42187f91..5e9711a4a 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -1051,4 +1051,121 @@ BOOST_AUTO_TEST_CASE(script_GetScriptAsm) BOOST_CHECK_EQUAL(derSig + "83 " + pubKey, ScriptToAsmStr(CScript() << ToByteVector(ParseHex(derSig + "83")) << vchPubKey)); } +static CScript +ScriptFromHex(const char* hex) +{ + std::vector data = ParseHex(hex); + return CScript(data.begin(), data.end()); +} + + +BOOST_AUTO_TEST_CASE(script_FindAndDelete) +{ + // Exercise the FindAndDelete functionality + CScript s; + CScript d; + CScript expect; + + s = CScript() << OP_1 << OP_2; + d = CScript(); // delete nothing should be a no-op + expect = s; + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 0); + BOOST_CHECK(s == expect); + + s = CScript() << OP_1 << OP_2 << OP_3; + d = CScript() << OP_2; + expect = CScript() << OP_1 << OP_3; + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 1); + BOOST_CHECK(s == expect); + + s = CScript() << OP_3 << OP_1 << OP_3 << OP_3 << OP_4 << OP_3; + d = CScript() << OP_3; + expect = CScript() << OP_1 << OP_4; + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 4); + BOOST_CHECK(s == expect); + + s = ScriptFromHex("0302ff03"); // PUSH 0x02ff03 onto stack + d = ScriptFromHex("0302ff03"); + expect = CScript(); + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 1); + BOOST_CHECK(s == expect); + + s = ScriptFromHex("0302ff030302ff03"); // PUSH 0x2ff03 PUSH 0x2ff03 + d = ScriptFromHex("0302ff03"); + expect = CScript(); + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 2); + BOOST_CHECK(s == expect); + + s = ScriptFromHex("0302ff030302ff03"); + d = ScriptFromHex("02"); + expect = s; // FindAndDelete matches entire opcodes + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 0); + BOOST_CHECK(s == expect); + + s = ScriptFromHex("0302ff030302ff03"); + d = ScriptFromHex("ff"); + expect = s; + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 0); + BOOST_CHECK(s == expect); + + // This is an odd edge case: strip of the push-three-bytes + // prefix, leaving 02ff03 which is push-two-bytes: + s = ScriptFromHex("0302ff030302ff03"); + d = ScriptFromHex("03"); + expect = CScript() << ParseHex("ff03") << ParseHex("ff03"); + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 2); + BOOST_CHECK(s == expect); + + // Byte sequence that spans multiple opcodes: + s = ScriptFromHex("02feed5169"); // PUSH(0xfeed) OP_1 OP_VERIFY + d = ScriptFromHex("feed51"); + expect = s; + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 0); // doesn't match 'inside' opcodes + BOOST_CHECK(s == expect); + + s = ScriptFromHex("02feed5169"); // PUSH(0xfeed) OP_1 OP_VERIFY + d = ScriptFromHex("02feed51"); + expect = ScriptFromHex("69"); + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 1); + BOOST_CHECK(s == expect); + + s = ScriptFromHex("516902feed5169"); + d = ScriptFromHex("feed51"); + expect = s; + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 0); + BOOST_CHECK(s == expect); + + s = ScriptFromHex("516902feed5169"); + d = ScriptFromHex("02feed51"); + expect = ScriptFromHex("516969"); + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 1); + BOOST_CHECK(s == expect); + + s = CScript() << OP_0 << OP_0 << OP_1 << OP_1; + d = CScript() << OP_0 << OP_1; + expect = CScript() << OP_0 << OP_1; // FindAndDelete is single-pass + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 1); + BOOST_CHECK(s == expect); + + s = CScript() << OP_0 << OP_0 << OP_1 << OP_0 << OP_1 << OP_1; + d = CScript() << OP_0 << OP_1; + expect = CScript() << OP_0 << OP_1; // FindAndDelete is single-pass + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 2); + BOOST_CHECK(s == expect); + + // Another weird edge case: + // End with invalid push (not enough data)... + s = ScriptFromHex("0003feed"); + d = ScriptFromHex("03feed"); // ... can remove the invalid push + expect = ScriptFromHex("00"); + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 1); + BOOST_CHECK(s == expect); + + s = ScriptFromHex("0003feed"); + d = ScriptFromHex("00"); + expect = ScriptFromHex("03feed"); + BOOST_CHECK_EQUAL(s.FindAndDelete(d), 1); + BOOST_CHECK(s == expect); +} + BOOST_AUTO_TEST_SUITE_END() From d1d7775587473410a107e7079616b9ecaae8dd06 Mon Sep 17 00:00:00 2001 From: Patrick Strateman Date: Tue, 19 Apr 2016 13:17:38 -0700 Subject: [PATCH 4/4] Improve worst-case behavior of CScript::FindAndDelete Thanks to Sergio Lerner for identifying this issue and suggesting this kind of solution. --- src/script/script.h | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/script/script.h b/src/script/script.h index 0503b39a7..bdbd340bc 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -570,17 +570,26 @@ public: int nFound = 0; if (b.empty()) return nFound; - iterator pc = begin(); + CScript result; + iterator pc = begin(), pc2 = begin(); opcodetype opcode; do { + result.insert(result.end(), pc2, pc); while (static_cast(end() - pc) >= b.size() && std::equal(b.begin(), b.end(), pc)) { - pc = erase(pc, pc + b.size()); + pc = pc + b.size(); ++nFound; } + pc2 = pc; } while (GetOp(pc, opcode)); + + if (nFound > 0) { + result.insert(result.end(), pc2, end()); + *this = result; + } + return nFound; } int Find(opcodetype op) const