Page MenuHomePhabricator

Introduce BlockHash to represent a block hash
ClosedPublic

Authored by deadalnix on Sat, Nov 30, 03:01.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC1450d42778b0: Introduce BlockHash to represent a block hash
Summary

So far we've been using raw uint256 for block hashes. Many things in bitcoin are uint256, such as transaction ids, merkle root, sighash, and so on.

Using a dedicated type ensures greater type safety and allows to make API more specific.

Test Plan
make check
./test/functional/test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

deadalnix created this revision.Sat, Nov 30, 03:01
Herald added a reviewer: Restricted Project. · View Herald TranscriptSat, Nov 30, 03:01
deadalnix updated this revision to Diff 14527.Sat, Nov 30, 03:03

Update copyright year

Fabien requested changes to this revision.Sat, Nov 30, 14:49
Fabien added a subscriber: Fabien.
Fabien added inline comments.
contrib/devtools/chainparams/generate_chainparams_constants.py
52 ↗(On Diff #14527)

Keep for chainwork

56 ↗(On Diff #14527)

Chainwork is not a block hash

59 ↗(On Diff #14527)

Dito

src/consensus/params.h
9 ↗(On Diff #14527)

Even if included indirectly by primitives/blockhash.h, you should keep it for nMinimumChainWork so that if won't break the build if the inclusion is removed from primitives/blockhash.h.

src/primitives/block.h
11 ↗(On Diff #14527)

Keep for hashMerkleRoot

src/rpc/blockchain.cpp
826 ↗(On Diff #14527)

Can use fromHex()

937 ↗(On Diff #14527)

Dito

1561 ↗(On Diff #14527)

Dito

1598 ↗(On Diff #14527)

Dito

1639 ↗(On Diff #14527)

That will be the same for all RPCs

src/test/coins_tests.cpp
9 ↗(On Diff #14527)

Missing <primitives/blockhash.h> ?

src/wallet/rpcwallet.cpp
2675 ↗(On Diff #14527)

Use fromHex()

This revision now requires changes to proceed.Sat, Nov 30, 14:49
deadalnix added inline comments.Sat, Nov 30, 16:31
contrib/devtools/chainparams/generate_chainparams_constants.py
52 ↗(On Diff #14527)

Good catch.

56 ↗(On Diff #14527)

You are correct

src/rpc/blockchain.cpp
826 ↗(On Diff #14527)

It's just more verbose, and does the same.

src/test/coins_tests.cpp
9 ↗(On Diff #14527)

It's not needed.

Fabien accepted this revision.Sat, Nov 30, 17:07
This revision is now accepted and ready to land.Sat, Nov 30, 17:07
This revision was automatically updated to reflect the committed changes.