Page MenuHomePhabricator

Modify uint256's comparison to use proper endianess
ClosedPublic

Authored by deadalnix on Jul 1 2018, 14:15.

Details

Summary

This makes the comparison consistent with arith_uint256 .

Test Plan

Updated unit tests.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
uint256endian
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2780
Build 3670: Bitcoin ABC Buildbot (legacy)
Build 3669: arc lint + arc unit

Event Timeline

schancel requested changes to this revision.Jul 2 2018, 15:30
schancel added a subscriber: schancel.
schancel added inline comments.
src/uint256.h
44

These are big endian and this is to ensure they're compared from smallest to largest byte?

This revision now requires changes to proceed.Jul 2 2018, 15:30
jasonbcox added inline comments.
src/test/uint256_tests.cpp
129

What's the purpose of these bitshifts?

deadalnix added inline comments.
src/test/uint256_tests.cpp
129

Creating larger and larger values.

src/uint256.h
44

They are little endian.

jasonbcox requested changes to this revision.Jul 5 2018, 21:44
jasonbcox added inline comments.
src/uint256.h
43

Can't this Compare() function take any size of base_blob? If so, this line will segfault if other is smaller than this. Maybe the first check should be if (sizeof(data) == sizeof(other.data)) {

This revision now requires changes to proceed.Jul 5 2018, 21:44
deadalnix added inline comments.
src/uint256.h
43

base_blob is a template. it cannot be any size.

schancel added inline comments.
src/uint256.h
44

Oh right, the MSB should be compared first. Sorry.

This revision is now accepted and ready to land.Jul 10 2018, 15:52
This revision was automatically updated to reflect the committed changes.