Page MenuHomePhabricator

Include unverified UTXO commitment in Coinbase on generate
AbandonedPublic

Authored by jasonbcox on May 31 2018, 14:41.

Details

Reviewers
tomtomtom7
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Project
Summary

This includes the UTXO commitment in a coinbase output as per the
specification at
https://github.com/tomasvdw/bips/blob/master/ecmh-utxo-commitment-0.mediawiki

This only modifies the full nodes own "generate" function which is only used for
testing.

The getblocktemplate isn't yet effected.

Depends on D1117

Test Plan

Tests included

Diff Detail

Repository
rABC Bitcoin ABC
Branch
utxof
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 2691
Build 3494: Bitcoin ABC Buildbot (legacy)
Build 3493: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.May 31 2018, 14:41
jasonbcox requested changes to this revision.Jun 17 2018, 02:06
jasonbcox added a subscriber: jasonbcox.
jasonbcox added inline comments.
src/miner.cpp
40

The last char is a zero instead of an 'O'. Is that intended?

255

OP_PUSHDATA1,2,4 are 0x4c, 0x4d, 0x4e. What is 0x24? If the comment is inaccurate, please reword.

This revision now requires changes to proceed.Jun 17 2018, 02:06
Mengerian added inline comments.
src/miner.cpp
255

@jasonbcox Op Codes 0x01 to 0x4b (1-75) mean that number of bytes get pushed to the stack.

So in this case, op code 0x24 means the next 36 bytes get pushed to the stack.

A good reference list of Op Codes here: https://en.bitcoin.it/wiki/Script

src/miner.cpp
255

Perhaps this comment would better serve as something like: // push 36 bytes on the stack

Even better, it should detail what those bytes expect to contain. Looking below, it appears to be some number of bytes for the magic (4-bytes according to src/miner.cpp) + 32 bytes for the commitment.

Additionally, this block of code will blindly accept longer magic values and commitment hashes, yet the push value here is hard-coded. Please fix this.

256

Include expected prefix length (4-bytes) in the comment. Either enforce it or change the bytes-to-push value to be dynamic based on the input.

260

Include expected commitment length (32-bytes). Either enforce it or change the bytes-to-push value to be dynamic based on the input.

jasonbcox abandoned this revision.
jasonbcox edited reviewers, added: tomtomtom7; removed: jasonbcox.