Page MenuHomePhabricator

Introduce BlockHash to represent a block hash
ClosedPublic

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

Details

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
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Fabien requested changes to this revision.Nov 30 2019, 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.Nov 30 2019, 14:49
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.

This revision is now accepted and ready to land.Nov 30 2019, 17:07