From 7b1acda395879f4ff4b430647b98328f785ba2e4 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 26 Apr 2016 14:34:40 +0200 Subject: [PATCH 1/9] chain: Add assertion in case of missing records in index db --- src/chain.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/chain.cpp b/src/chain.cpp index 5b8ce076c..39520cc8f 100644 --- a/src/chain.cpp +++ b/src/chain.cpp @@ -90,6 +90,7 @@ CBlockIndex* CBlockIndex::GetAncestor(int height) pindexWalk = pindexWalk->pskip; heightWalk = heightSkip; } else { + assert(pindexWalk->pprev); pindexWalk = pindexWalk->pprev; heightWalk--; } From 54a97348979ac58cfb89ff65de69ef75d4ecaf10 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 27 Apr 2016 11:07:43 +0200 Subject: [PATCH 2/9] test: Add more thorough test for dbwrapper iterators I made a silly mistake in a database wrapper where keys were sorted by char instead of uint8_t. As x86 char is signed the sorting for the block index database was messed up, resulting in a segfault due to missing records. Add a test to catch: - Wrong sorting - Seeking errors - Iteration result not complete --- src/test/dbwrapper_tests.cpp | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 496ae4be5..2a4f15114 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -116,5 +116,39 @@ BOOST_AUTO_TEST_CASE(dbwrapper_iterator) BOOST_CHECK_EQUAL(it->Valid(), false); } } - + +BOOST_AUTO_TEST_CASE(iterator_ordering) +{ + path ph = temp_directory_path() / unique_path(); + CDBWrapper dbw(ph, (1 << 20), true, false); + for (int x=0x00; x<256; ++x) { + uint8_t key = x; + uint32_t value = x*x; + BOOST_CHECK(dbw.Write(key, value)); + } + + boost::scoped_ptr it(const_cast(&dbw)->NewIterator()); + for (int c=0; c<2; ++c) { + int seek_start; + if (c == 0) + seek_start = 0x00; + else + seek_start = 0x80; + it->Seek((uint8_t)seek_start); + for (int x=seek_start; x<256; ++x) { + uint8_t key; + uint32_t value; + BOOST_CHECK(it->Valid()); + if (!it->Valid()) // Avoid spurious errors about invalid iterator's key and value in case of failure + break; + BOOST_CHECK(it->GetKey(key)); + BOOST_CHECK(it->GetValue(value)); + BOOST_CHECK_EQUAL(key, x); + BOOST_CHECK_EQUAL(value, x*x); + it->Next(); + } + BOOST_CHECK(!it->Valid()); + } +} + BOOST_AUTO_TEST_SUITE_END() From cf00779278827e2b5737c6261c23ac7cdf0580a4 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 2 May 2016 17:52:31 -0700 Subject: [PATCH 3/9] Add test for dbwrapper iterators with same-prefix keys. --- src/test/dbwrapper_tests.cpp | 86 ++++++++++++++++++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 2a4f15114..98d3f4814 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -151,4 +151,90 @@ BOOST_AUTO_TEST_CASE(iterator_ordering) } } +struct StringContentsSerializer { + // Used to make two serialized objects the same while letting them have a different lengths + // This is a terrible idea + string str; + StringContentsSerializer() {} + StringContentsSerializer(const string& inp) : str(inp) {} + + StringContentsSerializer& operator+=(const string& s) { + str += s; + return *this; + } + StringContentsSerializer& operator+=(const StringContentsSerializer& s) { return *this += s.str; } + + ADD_SERIALIZE_METHODS; + + template + inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { + if (ser_action.ForRead()) { + str.clear(); + char c = 0; + while (true) { + try { + READWRITE(c); + str.push_back(c); + } catch (const std::ios_base::failure& e) { + break; + } + } + } else { + for (size_t i = 0; i < str.size(); i++) + READWRITE(str[i]); + } + } +}; + +BOOST_AUTO_TEST_CASE(iterator_string_ordering) +{ + char buf[10]; + + path ph = temp_directory_path() / unique_path(); + CDBWrapper dbw(ph, (1 << 20), true, false); + for (int x=0x00; x<10; ++x) { + for (int y = 0; y < 10; y++) { + sprintf(buf, "%d", x); + StringContentsSerializer key(buf); + for (int z = 0; z < y; z++) + key += key; + uint32_t value = x*x; + BOOST_CHECK(dbw.Write(key, value)); + } + } + + boost::scoped_ptr it(const_cast(&dbw)->NewIterator()); + for (int c=0; c<2; ++c) { + int seek_start; + if (c == 0) + seek_start = 0; + else + seek_start = 5; + sprintf(buf, "%d", seek_start); + StringContentsSerializer seek_key(buf); + it->Seek(seek_key); + for (int x=seek_start; x<10; ++x) { + for (int y = 0; y < 10; y++) { + sprintf(buf, "%d", x); + string exp_key(buf); + for (int z = 0; z < y; z++) + exp_key += exp_key; + StringContentsSerializer key; + uint32_t value; + BOOST_CHECK(it->Valid()); + if (!it->Valid()) // Avoid spurious errors about invalid iterator's key and value in case of failure + break; + BOOST_CHECK(it->GetKey(key)); + BOOST_CHECK(it->GetValue(value)); + BOOST_CHECK_EQUAL(key.str, exp_key); + BOOST_CHECK_EQUAL(value, x*x); + it->Next(); + } + } + BOOST_CHECK(!it->Valid()); + } +} + + + BOOST_AUTO_TEST_SUITE_END() From 162cc4240e190d49a32919ec6adcb01e8aeafeb6 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 4 Apr 2018 14:59:07 +0100 Subject: [PATCH 4/9] Fix typo --- src/test/dbwrapper_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 98d3f4814..af44104cb 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -152,7 +152,7 @@ BOOST_AUTO_TEST_CASE(iterator_ordering) } struct StringContentsSerializer { - // Used to make two serialized objects the same while letting them have a different lengths + // Used to make two serialized objects the same while letting them have different lengths // This is a terrible idea string str; StringContentsSerializer() {} From 64b4790789f5519e397210095a1139a63c6a539a Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sun, 26 Feb 2017 21:08:26 +0100 Subject: [PATCH 5/9] test: Replace remaining sprintf with snprintf Use of `sprintf` is seen as a red flag as many of its uses are insecure. OpenBSD warns about it while compiling, and some modern platforms, e.g. [cloudlibc from cloudabi](https://github.com/NuxiNL/cloudlibc) don't even provide it anymore. Although our uses of these functions are secure, it can't hurt to replace them anyway. There are only 3 occurences left, all in the tests. --- src/test/dbwrapper_tests.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index af44104cb..2c6943516 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -194,7 +194,7 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) CDBWrapper dbw(ph, (1 << 20), true, false); for (int x=0x00; x<10; ++x) { for (int y = 0; y < 10; y++) { - sprintf(buf, "%d", x); + snprintf(buf, sizeof(buf), "%d", x); StringContentsSerializer key(buf); for (int z = 0; z < y; z++) key += key; @@ -210,12 +210,12 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) seek_start = 0; else seek_start = 5; - sprintf(buf, "%d", seek_start); + snprintf(buf, sizeof(buf), "%d", seek_start); StringContentsSerializer seek_key(buf); it->Seek(seek_key); for (int x=seek_start; x<10; ++x) { for (int y = 0; y < 10; y++) { - sprintf(buf, "%d", x); + snprintf(buf, sizeof(buf), "%d", x); string exp_key(buf); for (int z = 0; z < y; z++) exp_key += exp_key; From 923f2579b10ff3e7fce5561114c9c773a4ef5702 Mon Sep 17 00:00:00 2001 From: Lauda Date: Fri, 13 Jan 2017 16:05:16 +0100 Subject: [PATCH 6/9] [Trivial] Grammar and typo correction Minor corrections in src\test\* . --- src/test/dbwrapper_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 2c6943516..30babb565 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -73,7 +73,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch) BOOST_CHECK(dbw.Read(key2, res)); BOOST_CHECK_EQUAL(res.ToString(), in2.ToString()); - // key3 never should've been written + // key3 should've never been written BOOST_CHECK(dbw.Read(key3, res) == false); } } From 2742b208330a853a8eee9e1773825a38943933a0 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 4 Apr 2018 09:57:29 -0700 Subject: [PATCH 7/9] Use range based for loop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of iterating over 0 .. 1 and then deciding on an actual desired value, use a range based for loop for the desired value. Adapted from d0413c670b4e5dc79d5cc1bc35571fca745c9a24 Authored-by: René Nyffenegger --- src/test/dbwrapper_tests.cpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index 30babb565..f5e974a13 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -128,12 +128,7 @@ BOOST_AUTO_TEST_CASE(iterator_ordering) } boost::scoped_ptr it(const_cast(&dbw)->NewIterator()); - for (int c=0; c<2; ++c) { - int seek_start; - if (c == 0) - seek_start = 0x00; - else - seek_start = 0x80; + for (int seek_start : {0x00, 0x80}) { it->Seek((uint8_t)seek_start); for (int x=seek_start; x<256; ++x) { uint8_t key; @@ -204,12 +199,7 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) } boost::scoped_ptr it(const_cast(&dbw)->NewIterator()); - for (int c=0; c<2; ++c) { - int seek_start; - if (c == 0) - seek_start = 0; - else - seek_start = 5; + for (int seek_start : {0, 5}) { snprintf(buf, sizeof(buf), "%d", seek_start); StringContentsSerializer seek_key(buf); it->Seek(seek_key); From 7cc423522f3497c374670eaccd11433fd9db11f9 Mon Sep 17 00:00:00 2001 From: Simon Date: Wed, 4 Apr 2018 10:16:06 -0700 Subject: [PATCH 8/9] Bump MIT Licence copyright header. --- src/test/dbwrapper_tests.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index f5e974a13..a23d646ab 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -1,4 +1,5 @@ -// Copyright (c) 2012-2013 The Bitcoin Core developers +// Copyright (c) 2018 The Zcash developers +// Copyright (c) 2012-2017 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. From 8a22734ec1ee950f5e3f1279d4a636aafe29a977 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Wed, 4 Apr 2018 19:18:23 +0100 Subject: [PATCH 9/9] test: Check return value of snprintf --- src/test/dbwrapper_tests.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/dbwrapper_tests.cpp b/src/test/dbwrapper_tests.cpp index a23d646ab..c9b119ab2 100644 --- a/src/test/dbwrapper_tests.cpp +++ b/src/test/dbwrapper_tests.cpp @@ -190,7 +190,8 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) CDBWrapper dbw(ph, (1 << 20), true, false); for (int x=0x00; x<10; ++x) { for (int y = 0; y < 10; y++) { - snprintf(buf, sizeof(buf), "%d", x); + int n = snprintf(buf, sizeof(buf), "%d", x); + assert(n > 0 && n < sizeof(buf)); StringContentsSerializer key(buf); for (int z = 0; z < y; z++) key += key; @@ -201,12 +202,14 @@ BOOST_AUTO_TEST_CASE(iterator_string_ordering) boost::scoped_ptr it(const_cast(&dbw)->NewIterator()); for (int seek_start : {0, 5}) { - snprintf(buf, sizeof(buf), "%d", seek_start); + int n = snprintf(buf, sizeof(buf), "%d", seek_start); + assert(n > 0 && n < sizeof(buf)); StringContentsSerializer seek_key(buf); it->Seek(seek_key); for (int x=seek_start; x<10; ++x) { for (int y = 0; y < 10; y++) { - snprintf(buf, sizeof(buf), "%d", x); + int n = snprintf(buf, sizeof(buf), "%d", x); + assert(n > 0 && n < sizeof(buf)); string exp_key(buf); for (int z = 0; z < y; z++) exp_key += exp_key;