Skip to content

Commit fcdb39d

Browse files
fanquakefanquake
committed
Merge bitcoin#28924: refactor: Remove unused and fragile string interface from arith_uint256
fa63f16 test: Add uint256 string parse tests (MarcoFalke) facf629 refactor: Remove unused and fragile string interface from arith_uint256 (MarcoFalke) Pull request description: The string interface (`base_uint(const std::string&)`, as well as `base_uint::SetHex`) is problematic for many reasons: * It is unused (except in test-only code). * It is redundant with the `uint256` string interface: `std::string -> uint256 -> UintToArith256`. * It is brittle, because it inherits the brittle `uint256` string interface, which is brittle due to the use of `c_str()` (embedded null will be treated as end-of string), etc ... Instead of fixing the interface, remove it since it is unused and redundant with `UintToArith256`. ACKs for top commit: ajtowns: ACK fa63f16 TheCharlatan: ACK fa63f16 Tree-SHA512: a95d5b938ffd0473361336bbf6be093d01265a626c50be1345ce2c5e582c0f3f73eb11af5fd1884019f59d7ba27e670ecffdb41d2c624ffb9aa63bd52b780e62
2 parents dce1dfb + fa63f16 commit fcdb39d

File tree

5 files changed

+56
-52
lines changed

5 files changed

+56
-52
lines changed

‎src/arith_uint256.cpp

Lines changed: 1 addition & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,7 @@
88
#include <uint256.h>
99
#include <crypto/common.h>
1010

11-
12-
template <unsigned int BITS>
13-
base_uint<BITS>::base_uint(const std::string& str)
14-
{
15-
static_assert(BITS/32 > 0 && BITS%32 == 0, "Template parameter BITS must be a positive multiple of 32.");
16-
17-
SetHex(str);
18-
}
11+
#include <cassert>
1912

2013
template <unsigned int BITS>
2114
base_uint<BITS>& base_uint<BITS>::operator<<=(unsigned int shift)
@@ -153,22 +146,6 @@ std::string base_uint<BITS>::GetHex() const
153146
return b.GetHex();
154147
}
155148

156-
template <unsigned int BITS>
157-
void base_uint<BITS>::SetHex(const char* psz)
158-
{
159-
base_blob<BITS> b;
160-
b.SetHex(psz);
161-
for (int x = 0; x < this->WIDTH; ++x) {
162-
this->pn[x] = ReadLE32(b.begin() + x*4);
163-
}
164-
}
165-
166-
template <unsigned int BITS>
167-
void base_uint<BITS>::SetHex(const std::string& str)
168-
{
169-
SetHex(str.c_str());
170-
}
171-
172149
template <unsigned int BITS>
173150
std::string base_uint<BITS>::ToString() const
174151
{

‎src/arith_uint256.h

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,10 @@
66
#ifndef BITCOIN_ARITH_UINT256_H
77
#define BITCOIN_ARITH_UINT256_H
88

9+
#include <cstdint>
910
#include <cstring>
1011
#include <limits>
1112
#include <stdexcept>
12-
#include <stdint.h>
1313
#include <string>
1414

1515
class uint256;
@@ -56,8 +56,6 @@ class base_uint
5656
pn[i] = 0;
5757
}
5858

59-
explicit base_uint(const std::string& str);
60-
6159
base_uint operator~() const
6260
{
6361
base_uint ret;
@@ -219,8 +217,6 @@ class base_uint
219217
friend inline bool operator!=(const base_uint& a, uint64_t b) { return !a.EqualTo(b); }
220218

221219
std::string GetHex() const;
222-
void SetHex(const char* psz);
223-
void SetHex(const std::string& str);
224220
std::string ToString() const;
225221

226222
unsigned int size() const
@@ -247,7 +243,6 @@ class arith_uint256 : public base_uint<256> {
247243
arith_uint256() {}
248244
arith_uint256(const base_uint<256>& b) : base_uint<256>(b) {}
249245
arith_uint256(uint64_t b) : base_uint<256>(b) {}
250-
explicit arith_uint256(const std::string& str) : base_uint<256>(str) {}
251246

252247
/**
253248
* The "compact" format is a representation of a whole

‎src/test/arith_uint256_tests.cpp

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ static inline arith_uint256 arith_uint256V(const std::vector<unsigned char>& vch
2222
{
2323
return UintToArith256(uint256(vch));
2424
}
25+
static inline arith_uint256 arith_uint256S(const std::string& str) { return UintToArith256(uint256S(str)); }
2526

2627
const unsigned char R1Array[] =
2728
"\x9c\x52\x4a\xdb\xcf\x56\x11\x12\x2b\x29\x12\x5e\x5d\x35\xd2\xd2"
@@ -95,25 +96,25 @@ BOOST_AUTO_TEST_CASE( basics ) // constructors, equality, inequality
9596
BOOST_CHECK(ZeroL == (OneL << 256));
9697

9798
// String Constructor and Copy Constructor
98-
BOOST_CHECK(arith_uint256("0x"+R1L.ToString()) == R1L);
99-
BOOST_CHECK(arith_uint256("0x"+R2L.ToString()) == R2L);
100-
BOOST_CHECK(arith_uint256("0x"+ZeroL.ToString()) == ZeroL);
101-
BOOST_CHECK(arith_uint256("0x"+OneL.ToString()) == OneL);
102-
BOOST_CHECK(arith_uint256("0x"+MaxL.ToString()) == MaxL);
103-
BOOST_CHECK(arith_uint256(R1L.ToString()) == R1L);
104-
BOOST_CHECK(arith_uint256(" 0x"+R1L.ToString()+" ") == R1L);
105-
BOOST_CHECK(arith_uint256("") == ZeroL);
106-
BOOST_CHECK(R1L == arith_uint256(R1ArrayHex));
99+
BOOST_CHECK(arith_uint256S("0x" + R1L.ToString()) == R1L);
100+
BOOST_CHECK(arith_uint256S("0x" + R2L.ToString()) == R2L);
101+
BOOST_CHECK(arith_uint256S("0x" + ZeroL.ToString()) == ZeroL);
102+
BOOST_CHECK(arith_uint256S("0x" + OneL.ToString()) == OneL);
103+
BOOST_CHECK(arith_uint256S("0x" + MaxL.ToString()) == MaxL);
104+
BOOST_CHECK(arith_uint256S(R1L.ToString()) == R1L);
105+
BOOST_CHECK(arith_uint256S(" 0x" + R1L.ToString() + " ") == R1L);
106+
BOOST_CHECK(arith_uint256S("") == ZeroL);
107+
BOOST_CHECK(R1L == arith_uint256S(R1ArrayHex));
107108
BOOST_CHECK(arith_uint256(R1L) == R1L);
108109
BOOST_CHECK((arith_uint256(R1L^R2L)^R2L) == R1L);
109110
BOOST_CHECK(arith_uint256(ZeroL) == ZeroL);
110111
BOOST_CHECK(arith_uint256(OneL) == OneL);
111112

112113
// uint64_t constructor
113-
BOOST_CHECK( (R1L & arith_uint256("0xffffffffffffffff")) == arith_uint256(R1LLow64));
114+
BOOST_CHECK((R1L & arith_uint256S("0xffffffffffffffff")) == arith_uint256(R1LLow64));
114115
BOOST_CHECK(ZeroL == arith_uint256(0));
115116
BOOST_CHECK(OneL == arith_uint256(1));
116-
BOOST_CHECK(arith_uint256("0xffffffffffffffff") == arith_uint256(0xffffffffffffffffULL));
117+
BOOST_CHECK(arith_uint256S("0xffffffffffffffff") == arith_uint256(0xffffffffffffffffULL));
117118

118119
// Assignment (from base_uint)
119120
arith_uint256 tmpL = ~ZeroL; BOOST_CHECK(tmpL == ~ZeroL);
@@ -282,7 +283,7 @@ BOOST_AUTO_TEST_CASE( comparison ) // <= >= < >
282283
BOOST_AUTO_TEST_CASE( plusMinus )
283284
{
284285
arith_uint256 TmpL = 0;
285-
BOOST_CHECK(R1L+R2L == arith_uint256(R1LplusR2L));
286+
BOOST_CHECK(R1L + R2L == arith_uint256S(R1LplusR2L));
286287
TmpL += R1L;
287288
BOOST_CHECK(TmpL == R1L);
288289
TmpL += R2L;
@@ -346,8 +347,8 @@ BOOST_AUTO_TEST_CASE( multiply )
346347

347348
BOOST_AUTO_TEST_CASE( divide )
348349
{
349-
arith_uint256 D1L("AD7133AC1977FA2B7");
350-
arith_uint256 D2L("ECD751716");
350+
arith_uint256 D1L{arith_uint256S("AD7133AC1977FA2B7")};
351+
arith_uint256 D2L{arith_uint256S("ECD751716")};
351352
BOOST_CHECK((R1L / D1L).ToString() == "00000000000000000b8ac01106981635d9ed112290f8895545a7654dde28fb3a");
352353
BOOST_CHECK((R1L / D2L).ToString() == "000000000873ce8efec5b67150bad3aa8c5fcb70e947586153bf2cec7c37c57a");
353354
BOOST_CHECK(R1L / OneL == R1L);
@@ -368,19 +369,22 @@ static bool almostEqual(double d1, double d2)
368369
return fabs(d1-d2) <= 4*fabs(d1)*std::numeric_limits<double>::epsilon();
369370
}
370371

371-
BOOST_AUTO_TEST_CASE( methods ) // GetHex SetHex size() GetLow64 GetSerializeSize, Serialize, Unserialize
372+
BOOST_AUTO_TEST_CASE(methods) // GetHex operator= size() GetLow64 GetSerializeSize, Serialize, Unserialize
372373
{
373374
BOOST_CHECK(R1L.GetHex() == R1L.ToString());
374375
BOOST_CHECK(R2L.GetHex() == R2L.ToString());
375376
BOOST_CHECK(OneL.GetHex() == OneL.ToString());
376377
BOOST_CHECK(MaxL.GetHex() == MaxL.ToString());
377378
arith_uint256 TmpL(R1L);
378379
BOOST_CHECK(TmpL == R1L);
379-
TmpL.SetHex(R2L.ToString()); BOOST_CHECK(TmpL == R2L);
380-
TmpL.SetHex(ZeroL.ToString()); BOOST_CHECK(TmpL == 0);
381-
TmpL.SetHex(HalfL.ToString()); BOOST_CHECK(TmpL == HalfL);
380+
TmpL = R2L;
381+
BOOST_CHECK(TmpL == R2L);
382+
TmpL = ZeroL;
383+
BOOST_CHECK(TmpL == 0);
384+
TmpL = HalfL;
385+
BOOST_CHECK(TmpL == HalfL);
382386

383-
TmpL.SetHex(R1L.ToString());
387+
TmpL = R1L;
384388
BOOST_CHECK(R1L.size() == 32);
385389
BOOST_CHECK(R2L.size() == 32);
386390
BOOST_CHECK(ZeroL.size() == 32);

‎src/test/pow_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ void sanity_check_chainparams(const ArgsManager& args, ChainType chain_type)
177177

178178
// check max target * 4*nPowTargetTimespan doesn't overflow -- see pow.cpp:CalculateNextWorkRequired()
179179
if (!consensus.fPowNoRetargeting) {
180-
arith_uint256 targ_max("0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF");
180+
arith_uint256 targ_max{UintToArith256(uint256S("0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"))};
181181
targ_max /= consensus.nPowTargetTimespan*4;
182182
BOOST_CHECK(UintToArith256(consensus.powLimit) < targ_max);
183183
}

‎src/test/uint256_tests.cpp

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,8 +259,8 @@ BOOST_AUTO_TEST_CASE( conversion )
259259
BOOST_CHECK(UintToArith256(OneL) == 1);
260260
BOOST_CHECK(ArithToUint256(0) == ZeroL);
261261
BOOST_CHECK(ArithToUint256(1) == OneL);
262-
BOOST_CHECK(arith_uint256(R1L.GetHex()) == UintToArith256(R1L));
263-
BOOST_CHECK(arith_uint256(R2L.GetHex()) == UintToArith256(R2L));
262+
BOOST_CHECK(arith_uint256(UintToArith256(uint256S(R1L.GetHex()))) == UintToArith256(R1L));
263+
BOOST_CHECK(arith_uint256(UintToArith256(uint256S(R2L.GetHex()))) == UintToArith256(R2L));
264264
BOOST_CHECK(R1L.GetHex() == UintToArith256(R1L).GetHex());
265265
BOOST_CHECK(R2L.GetHex() == UintToArith256(R2L).GetHex());
266266
}
@@ -278,6 +278,34 @@ BOOST_AUTO_TEST_CASE( operator_with_self )
278278
BOOST_CHECK(v == UintToArith256(uint256S("0")));
279279
}
280280

281+
BOOST_AUTO_TEST_CASE(parse)
282+
{
283+
{
284+
std::string s_12{"0000000000000000000000000000000000000000000000000000000000000012"};
285+
BOOST_CHECK_EQUAL(uint256S("12\0").GetHex(), s_12);
286+
BOOST_CHECK_EQUAL(uint256S(std::string{"12\0", 3}).GetHex(), s_12);
287+
BOOST_CHECK_EQUAL(uint256S("0x12").GetHex(), s_12);
288+
BOOST_CHECK_EQUAL(uint256S(" 0x12").GetHex(), s_12);
289+
BOOST_CHECK_EQUAL(uint256S(" 12").GetHex(), s_12);
290+
}
291+
{
292+
std::string s_1{uint256::ONE.GetHex()};
293+
BOOST_CHECK_EQUAL(uint256S("1\0").GetHex(), s_1);
294+
BOOST_CHECK_EQUAL(uint256S(std::string{"1\0", 2}).GetHex(), s_1);
295+
BOOST_CHECK_EQUAL(uint256S("0x1").GetHex(), s_1);
296+
BOOST_CHECK_EQUAL(uint256S(" 0x1").GetHex(), s_1);
297+
BOOST_CHECK_EQUAL(uint256S(" 1").GetHex(), s_1);
298+
}
299+
{
300+
std::string s_0{uint256::ZERO.GetHex()};
301+
BOOST_CHECK_EQUAL(uint256S("\0").GetHex(), s_0);
302+
BOOST_CHECK_EQUAL(uint256S(std::string{"\0", 1}).GetHex(), s_0);
303+
BOOST_CHECK_EQUAL(uint256S("0x").GetHex(), s_0);
304+
BOOST_CHECK_EQUAL(uint256S(" 0x").GetHex(), s_0);
305+
BOOST_CHECK_EQUAL(uint256S(" ").GetHex(), s_0);
306+
}
307+
}
308+
281309
BOOST_AUTO_TEST_CASE( check_ONE )
282310
{
283311
uint256 one = uint256S("0000000000000000000000000000000000000000000000000000000000000001");

0 commit comments

Comments
 (0)