Replace uint256/uint160 with opaque blobs where possible (cont'd)

laanwj commented at 5:43 pm on December 16, 2014: member

Second attempt at #5478. See discussion there for context.

This pull replaces almost all uses of uint256 and all uses of uint160 to use opaque byte blobs with only the following operations:

Default initialization to 0, or from a vector of bytes
Assignment from other blobXXXs
IsNull() compare to special all-zeros value
SetNull() clear to special all-zeros value: Bitcoin needs IsNull() / SetNull() because we often use the all-zeroes value as a marker for errors and empty values.
< for sorting in maps.
!= == test for (in)equality
GetHex/SetHex/ToString because they’re just so useful
begin()/end() raw access to the data
size() returns a fixed size
GetSerializeSize() Serialize Unserialize serialization just reads and writes the bytes
GetCheapHash() this is similar to GetLow64() and returns part of the value as uint64_t, for cheap hashing when the contents are assumed distributed uniformy random.

arith_uint256 (the old uint256, used for proof-of-work mainly), on the other hand, loses all functionality like raw bytes access and serialization. Its memory should not be accessed directly. This is necessary for #888 (big endian support). arith_uint160 (the old uint160) is completely removed as Bitcoin Core does no 160-bit integer arithmetic.

For overall steps see commits.

laanwj added the label Improvement on Dec 16, 2014

sipa commented at 0:02 am on December 17, 2014: member

Reviewed all commits up to f66f592aea94701a2fccb2a608e05f93f298da5b except b7155a9e96ced01651888cda89aee1a68d696852 and 958431c2c3f18a651a87d33cb79d9c2dc786374b.

sipa commented at 12:56 pm on December 18, 2014: member

Verified b7155a9e96ced01651888cda89aee1a68d696852’s code in aith_uint256.{h,cpp} to be a pure copy of the original code in uint256.{h,cpp}, apart from some type name changes, and IMPORTANT 2 deleted trailing white spaces in the comments above SetCompact.

sipa commented at 1:24 pm on December 18, 2014: member

Untested ACK, but the code (both blob and arithmetic versions of it) looks well tested.

laanwj commented at 12:19 pm on December 19, 2014: member

I’ve added a commit that (and left the rest untouched)

Removes direct arith_uint256 initialization from byte vector. It is only used in the tests, so I’ve provided an alternative function there that just goes through uint256.
Implements GetHex and SetHex in terms of uint256. As these functions work bytewise and expect little-endian they belong there (also removes a bit of duplication).

I’m not exactly sure about the latter. It is an improvement, but other options would be:

Reimplement GetHex and SetHex in terms of uint32_t’s instead of bytes, but keep them on arith_uint256
We could even get away with removing at least arith_uint256’s SetHex and initialization from strings completely (AFAIK also only used in the tests). It’s nice to be able to print a number though.

sipa commented at 0:54 am on December 30, 2014: member

Let’s not let this go too unrebasable…

sipa referenced this in commit 5dbc0fe712 on Dec 30, 2014

laanwj force-pushed on Dec 30, 2014

laanwj commented at 5:43 pm on December 30, 2014: member

Rebased for 78253fcbad69b28f1c0c3cfc7f8dcaecbb68c390 and d78f0dafd520f481f909cca7e361a4e482cbea72

gmaxwell commented at 10:36 am on January 1, 2015: contributor

ACK.

laanwj force-pushed on Jan 2, 2015

laanwj commented at 11:03 am on January 2, 2015: member

Rebased for c444c620c62c51d65f7de0b2a3c351e61ab1e388

sipa commented at 3:18 pm on January 2, 2015: member

@jgarzik @gavinandresen Care to review?

laanwj commented at 9:42 am on January 3, 2015: member

This isn’t catched by git, only by the compiler, but needs rebase for 012598880cf69a6a4d4d495c78e40ab4abb4eb39 as it introduces uint256(int)s in pmt_tests.

Temporarily add SetNull/IsNull/GetCheapHash to base_uint

Also add a stub for arith_uint256 and its conversion functions,
for now completely based on uint256.

Eases step-by-step migration to blob.

5d3064bc44

Replace direct use of 0 with SetNull and IsNull

Replace x=0 with .SetNull(),
x==0 with IsNull(), x!=0 with !IsNull().
Replace uses of uint256(0) with uint256().

4f1524966a

Replace GetLow64 with GetCheapHash 807658549c

Replace uint256(1) with static constant

SignatureHash and its test function SignatureHashOld
return uint256(1) as a special error signaling value.
Return a local static constant with the same value instead.

2eae3157f6

String conversions uint256 -> uint256S

If uint256() constructor takes a string, uint256(0) will become
dangerous when uint256 does not take integers anymore (it will go
through std::string(const char*) making a NULL string, and the explicit
keyword is no help).

34cdc41128

Use arith_uint256 where necessary

Also add conversion from/to uint256 where needed.

734f85c4f0

uint256->arith_uint256 blob256->uint256

Introduce new opaque implementation of `uint256`, move old
"arithmetic" implementation to `arith_uint256.

bfc6070342

Add conversion functions arith_uint256<->uint_256 92cdb1aace

Add tests for new uint256 dba2e9141a

Remove arith_uint160

We never do 160-bit arithmetic.

edc720479d

Remove now-unused methods from arith_uint256 and base_uint

- Methods that access the guts of arith_uint256 are removed,
as these are incompatible between endians. Use uint256 instead

- Serialization is no longer needed as arith_uint256's are never
read or written

- GetHash is never used on arith_uint256

30007fda76

arith_uint256: remove initialization from byte vector

Remove initialization from vector (as this is only used in the tests).

Also implement SetHex and GetHex in terms of uint256, to avoid
duplicate code as well as avoid endianness issues (as they
work in term of bytes).

6bd0dc2a84

laanwj force-pushed on Jan 5, 2015

laanwj commented at 3:03 pm on January 5, 2015: member

Rebased for 012598880cf69a6a4d4d495c78e40ab4abb4eb39. If Travis passes I’m just going to take the (minimal) risk and merge this, so that the big-endian patchset is easier to maintain.

laanwj merged this on Jan 5, 2015

laanwj closed this on Jan 5, 2015

laanwj referenced this in commit ec20fd74b8 on Jan 5, 2015

random-zebra referenced this in commit b3fcbea5b1 on Mar 17, 2020

akshaynexus referenced this in commit e124b7ee92 on Mar 22, 2020

MarcoFalke locked this on Sep 8, 2021

Replace uint256/uint160 with opaque blobs where possible (cont’d) #5490