From 8c959016092a2bdcd4d4d5ffe8e60303524562ea 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 54d98a078..1f7df2d61 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -549,7 +549,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 2bcf063b1daff4b701c44c500d64799ae3911158 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 1f7df2d61..de182cf79 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -549,7 +549,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 b0487957ad1868a39a4555917f15e7ffc6b3ada4 Mon Sep 17 00:00:00 2001 From: shaolinfry Date: Mon, 9 Jan 2017 11:25:26 +0000 Subject: [PATCH 3/4] Unit test for CScript::FindAndDelete Cherry-picked from bitcoin/e2a30bc9a9f7d2969e52632f8e8942a4e72f4ba6 credit gavin andresen --- 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 a084a145c..2dd0d0c33 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -971,4 +971,121 @@ BOOST_AUTO_TEST_CASE(script_IsPushOnly_on_invalid_scripts) BOOST_CHECK(!CScript(direct, direct+sizeof(direct)).IsPushOnly()); } +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 b30a81f3ad83fb8c40f5b4f72b70e175fb24f97f 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 de182cf79..35049573c 100644 --- a/src/script/script.h +++ b/src/script/script.h @@ -545,17 +545,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