Start differenciating txid and txhash
ClosedPublic
Actions

Authored by deadalnix on Dec 5 2017, 16:46.

Details

Reviewers

schancel
tomtomtom7
jasonbcox
dagurval

Group Reviewers

Restricted Project

Commits

rSTAGINGf71bd4dab0e5: Start differenciating txid and txhash
rABCf71bd4dab0e5: Start differenciating txid and txhash

Summary

Introduces the YxId and TxHash types. This will ensure type safety for cases requiring to use an id and cases requiring to use a hash.

To quote D665 :
This is the basis of MalFix with naming txhash/txid.

It is quite tricky to ensure no txid is kept where txhash is meant.
As txid will be rare, I propose for safety to fully rename txid to
txhash, eliminating txid usage before reintroducing txid to COutpoint.

Test Plan

make check

Diff Detail

Repository

rABC Bitcoin ABC

Branch

txidtxhash

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 1248
Build 1248: arc lint + arc unit

Event Timeline

deadalnix created this revision.Dec 5 2017, 16:46

Herald added a reviewer: Restricted Project. · View Herald TranscriptDec 5 2017, 16:46

deadalnix mentioned this in D665: Start refactoring txid -> txhash.Dec 5 2017, 16:48

LGTM

dagurval accepted this revision.Dec 5 2017, 20:19

This revision is now accepted and ready to land.Dec 5 2017, 20:19

Closed by commit rABCf71bd4dab0e5: Start differenciating txid and txhash (authored by tomtomtom7, committed by deadalnix). · Explain WhyDec 6 2017, 13:00

This revision was automatically updated to reflect the committed changes.

I am sorry for leaving this on table, but I must say I am bit disappointed about the procedure here.

I am aware some people like this idea, but don't we actually need support and agreement?

MalFix is a much more high impact change than the other things on ABC's and BU's midterm plan but it isn't there in the announcement; yet we're just going to put it in? Am I the only one who thinks that that is a bit odd?

I agree it seems premature. I'm not convinced it is solving a problem anyone has; should be low priority IMO until it's shown to be a) not useful in itself, and b) blocking other useful stuff people actually are asking to do. I've seen no evidence of either.