Add activation code for OP_CHECKDATASIG
ClosedPublic
Actions

Authored by deadalnix on Aug 5 2018, 20:55.

Details

Reviewers

jasonbcox
schancel

Group Reviewers

Restricted Project

Commits

Summary

As per title.

Depends on D1623 and D1621

Test Plan

Added an integration test to check for proper activation and deactivation.

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

jasonbcox requested changes to this revision.Aug 5 2018, 22:50

jasonbcox added inline comments.

src/validation.cpp
1928 ↗	(On Diff #4507)	acceptign -> accepting
2584 ↗	(On Diff #4507)	transactiosn -> transactions
test/functional/abc-checkdatasig-activation.py
76 ↗	(On Diff #4507)	OP_AND should be OP_CHECKDATASIG. very confusing otherwise.
87 ↗	(On Diff #4507)	spendable_ands -> spendable_checkdatasigs
101 ↗	(On Diff #4507)	opreturn -> something about checkdatasig
139 ↗	(On Diff #4507)	I'm confused how this magic number 6 was selected. With MTP being the last 11 blocks, does selecting fewer potentially make the test flaky and/or fragile to future changes, as there could be blocks mined before these 6 with unknown timestamps?
146 ↗	(On Diff #4507)	good check. I didn't think about testing the RPC in conjunction with the block acceptance. @schancel we should be on the lookout for stuff like this. in the future, a solid testing framework would be able to make checks like this for us.

This revision now requires changes to proceed.Aug 5 2018, 22:50

test/functional/abc-checkdatasig-activation.py
139 ↗	(On Diff #4507)	We are good for several decades.
146 ↗	(On Diff #4507)	Yes please.

Fix various typos and erroneous comments/variable names.

test/functional/abc-checkdatasig-activation.py
139 ↗	(On Diff #4507)	You're referring to the MAGNETIC_ANOMALY_START_TIME value, but I'm talking about MTP calculated using 11 blocks' worth of timestamps, not 6.

schancel requested changes to this revision.Aug 7 2018, 19:07

schancel added inline comments.

test/functional/abc-checkdatasig-activation.py
47 ↗	(On Diff #4519)	Can you please make this use self.node[0].calculate_fee() on the CTransaction so I don't have to go fix this? Is this even right since it's not multiplied by the size of the transaction?
71 ↗	(On Diff #4519)	:(