Page MenuHomePhabricator
Differential D1527

Modify uint256's comparison to use proper endianess
ClosedPublic
Actions

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

Details

Reviewers
schancel
jasonbcox
Group Reviewers
Restricted Project
Commits
rSTAGING7a5e1e55ec95: Modify uint256's comparison to use proper endianess
rABC7a5e1e55ec95: Modify uint256's comparison to use proper endianess
Summary

This makes the comparison consistent with arith_uint256 .

Test Plan

Updated unit tests.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix created this revision.Jul 1 2018, 14:15
Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 1 2018, 14:15
teamcity added a subscriber: teamcity.Jul 1 2018, 14:15

Build Bitcoin-ABC :: Diffs :: Bitcoin-ABC Diff Testing started.

teamcity added a comment.Jul 1 2018, 14:15

Build Bitcoin-ABC :: Diffs :: Bitcoin-ABC Diff Testing started.

teamcity added a comment.Jul 1 2018, 14:52

Build Bitcoin-ABC :: Diffs :: Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B2780: Diff 4174.Jul 1 2018, 14:53
deadalnix added a child revision: D1529: Implement canonical transaction ordering..Jul 1 2018, 19:13
deadalnix added a parent revision: D1526: Add support for both way comparison for uint256/uint160.Jul 1 2018, 19:13
schancel requested changes to this revision.Jul 2 2018, 15:30
schancel added a subscriber: schancel.
schancel added inline comments.
src/uint256.h
44 ↗(On Diff #4174)

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 a subscriber: jasonbcox.Jul 2 2018, 15:56
jasonbcox added inline comments.
src/test/uint256_tests.cpp
129 ↗(On Diff #4174)

What's the purpose of these bitshifts?

deadalnix requested review of this revision.Jul 2 2018, 22:53
deadalnix added inline comments.
src/test/uint256_tests.cpp
129 ↗(On Diff #4174)

Creating larger and larger values.

src/uint256.h
44 ↗(On Diff #4174)

They are little endian.

jasonbcox requested changes to this revision.Jul 5 2018, 21:44
jasonbcox added inline comments.
src/uint256.h
43 ↗(On Diff #4174)

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 requested review of this revision.Jul 6 2018, 11:46
deadalnix added inline comments.
src/uint256.h
43 ↗(On Diff #4174)

base_blob is a template. it cannot be any size.

schancel accepted this revision.Jul 6 2018, 16:44
schancel added inline comments.
src/uint256.h
44 ↗(On Diff #4174)

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

jasonbcox accepted this revision.Jul 10 2018, 15:52
This revision is now accepted and ready to land.Jul 10 2018, 15:52
Closed by commit rABC7a5e1e55ec95: Modify uint256's comparison to use proper endianess (authored by deadalnix). · Explain WhyJul 10 2018, 16:03
This revision was automatically updated to reflect the committed changes.
Fabien mentioned this in D9147: scripted-diff: rename base_blob::data to m_data.Feb 3 2021, 16:35
Fabien mentioned this in rABC676c36c107ae: scripted-diff: rename base_blob::data to m_data.Feb 4 2021, 08:50
Fabien mentioned this in D16491: [txpackages] add canonical way to get hash of package.Jul 17 2024, 18:07
Fabien mentioned this in rABC39a2132c1e7c: [txpackages] add canonical way to get hash of package.Jul 18 2024, 09:05

Revision Contents
Changeset List

PathSize
src/
test/
10 lines
15 lines

Diff 4253

View Options

src/test/uint256_tests.cpp

Loading...
View Options

src/uint256.h

Loading...