Include unverified UTXO commitment in Coinbase on generate
AbandonedPublic
Actions

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

Details

Reviewers

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

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

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.