Implement fork for "staking vaults" #38

domob1812 · 2020-09-28T13:11:24Z

Based on the op-code changes (OP_REQUIRE_COINSTAKE) by @galpHub, this finishes the consensus-parts of the planned fork to implement staking vaults. It introduces a new time-activated fork (currently at block time 2'000'000'000, to be scheduled with a follow-up change). The changes to consensus are as follows:

Blocks after the activation time will enforce OP_REQUIRE_COINSTAKE in signature verification.
They also enforce new rules on the structure of coinstakes: If a coinstake is spending a vault output, the coins used for staking and the staking reward have to be paid back to the same script in a single output.
From the fork, staking vault scripts are able to sign the blocks (in addition to P2PK and P2PKH scripts that are able to sign blocks before).
The extra signature verification done on the coinstake in CheckProofOfStake is no longer enforcing SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS, and it is also using OP_REQUIRE_COINSTAKE.

Note that 3) and 4) implement a hard fork, that is scheduled in parallel with the script changes which by themselves would only be a soft fork. This hard fork just lifts some rules that were stricter before; so once all is through and activated for long enough, we can retro-actively remove the stricter rules until genesis and simplify the code for 3) and 4).

galpHub

I'll give it another pass tomorrow as well. Overall looks great. Just need to review the code more closely.

divi/src/CoinMinter.cpp

divi/src/script/sign.cpp

divi/src/kernel.cpp

galpHub

So taking a look at the fixups, I think that - on second thought - maybe not so much a decorator of the CoinMinter but rather a decorator for the BlockFactory. I noticed that for PoS blocks the suggested changes will succeed but not so much for PoW blocks as it changes the merkle root.

In the constructor for the CoinMinter, since it receives the chain's parameters, you could instead do (pseudocode only for clarity)
blockFactory_( Params().IsRegTest() ? std::make_shared(...) : std::make_shared(...) )

divi/src/BlockFactory.cpp

divi/src/CoinMinter.cpp

domob1812 · 2020-10-01T12:24:29Z

I've now done some more follow-up changes (in 554cb5d and the preceding commits), which separate the changes to the CoinMinter and BlockFactory for generateblock off into a specific subclass used only in that place.

galpHub

Just a few rename suggestions.

divi/src/test/kernel_tests.cpp

domob1812 · 2020-10-23T08:16:16Z

I've applied the renaming changes in a fixup commit (will squash properly later when the review is done). I've also added a test per your suggestion for paying to another vault script.

divi/src/main.cpp

divi/src/script/standard.h

divi/src/kernel.cpp

domob1812 · 2020-11-04T15:46:51Z

We should update this PR to be based on the ActivationManager from #46 for fork activation.

EDIT: Done.

This defines a new fork, to be scheduled at a future block time, with these consensus changes: - Once activated, verify scripts with OP_REQUIRE_COINSTAKE. - In addition, blocks also apply the custom CheckCoinstakeForVaults verification routine to coinstake transactions (making sure that vault payments are correct, i.e. back to the vault with the right amount). For now, the fork is scheduled for UNIX time 2000000000 (more than a decade from now). This will be updated to the real time later, when the schedule has been finalised.

domob1812 · 2020-12-01T13:14:31Z

With the recent merge of #55 and #48, this is now quite simplified and ready for review and merging.

divi/src/BlockSigning.cpp

Extend the PoS block signing and verifying logic to accept also signatures from vault scripts; otherwise, staking with vaults will not actually be possible. Note that this is a hard fork. We activate it together with the other (soft fork) parts at the StakingVaults activation time.

The new regtest vaultfork.py verifies behaviour around activating the OP_REQUIRE_COINSTAKE fork with an activation time. Once the time has been reached, new blocks will enforce the post-fork rules (i.e. OP_REQUIRE_COINSTAKE and some extra coinstake checks).

In addition to normal signature verification done on transactions in blocks, CheckProofOfStake by itself already verifies the coinstake transaction's signature (presumably as some kind of DoS protection to fail early for blocks faked with someone else's stake). This verification uses SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS, which makes it impossible to implement upgrades like the vault fork (as well as future changes) as soft forks. This change introduces a hard fork activated together with Fork::StakingVaults, which removes this flag and thus allows coinstake scripts to contain upgradable NOPs. We also add SCRIPT_REQUIRE_COINSTAKE to the flags used in CheckProofOfStake. Since the transaction there is always a coinstake, this does not change anything except turn OP_REQUIRE_COINSTAKE from a NOP to a "real" opcode.

Add tests for vault scripts in real coinstakes of blocks in the vaultfork.py integration test. This verifies the extra conditions we apply to them after the fork (so the staker cannot steal coins from a vault) as well as verifies that proper staking as the vault is possible.

galpHub reviewed Sep 28, 2020

View reviewed changes

divi/src/CoinMinter.cpp Outdated Show resolved Hide resolved

divi/src/script/sign.cpp Outdated Show resolved Hide resolved

divi/src/kernel.cpp Show resolved Hide resolved

galpHub suggested changes Sep 30, 2020

View reviewed changes

galpHub reviewed Sep 30, 2020

View reviewed changes

divi/src/BlockFactory.cpp Outdated Show resolved Hide resolved

galpHub reviewed Sep 30, 2020

View reviewed changes

divi/src/BlockFactory.cpp Show resolved Hide resolved

galpHub suggested changes Sep 30, 2020

View reviewed changes

divi/src/CoinMinter.cpp Outdated Show resolved Hide resolved

domob1812 force-pushed the vault-upgrade branch 2 times, most recently from 984732a to 554cb5d Compare October 1, 2020 12:21

domob1812 force-pushed the vault-upgrade branch 3 times, most recently from a2007d7 to 440419d Compare October 5, 2020 13:25

domob1812 mentioned this pull request Oct 5, 2020

Some refactoring and add the "generateblock" RPC method #39

Merged

domob1812 force-pushed the vault-upgrade branch from 440419d to 60cf5be Compare October 5, 2020 14:04

domob1812 mentioned this pull request Oct 16, 2020

Custom reward addresses for masternodes #44

Closed

galpHub reviewed Oct 22, 2020

View reviewed changes

domob1812 force-pushed the vault-upgrade branch from 60cf5be to 3acad41 Compare October 22, 2020 14:43

galpHub reviewed Oct 28, 2020

View reviewed changes

divi/src/main.cpp Outdated Show resolved Hide resolved

divi/src/script/standard.h Outdated Show resolved Hide resolved

divi/src/kernel.cpp Outdated Show resolved Hide resolved

domob1812 mentioned this pull request Nov 4, 2020

Activation manager for BIP30-style forks #46

Closed

domob1812 force-pushed the vault-upgrade branch 2 times, most recently from b63373c to 90a7487 Compare November 5, 2020 15:00

domob1812 marked this pull request as draft November 5, 2020 15:02

domob1812 force-pushed the vault-upgrade branch 2 times, most recently from 00751fe to 78bc15c Compare November 6, 2020 15:31

domob1812 mentioned this pull request Nov 6, 2020

Extra check for staking vault coinstakes #48

Merged

domob1812 force-pushed the vault-upgrade branch 2 times, most recently from 2c687bf to a79419a Compare November 11, 2020 12:57

domob1812 force-pushed the vault-upgrade branch 2 times, most recently from 4254e7e to 5da18a6 Compare November 17, 2020 13:50

domob1812 force-pushed the vault-upgrade branch 13 times, most recently from a428971 to 08b4e3f Compare November 26, 2020 14:12

domob1812 force-pushed the vault-upgrade branch from 08b4e3f to 94159fd Compare November 26, 2020 15:06

domob1812 force-pushed the vault-upgrade branch from 94159fd to 70bd370 Compare December 1, 2020 13:12

domob1812 marked this pull request as ready for review December 1, 2020 13:13

galpHub suggested changes Dec 1, 2020

View reviewed changes

divi/src/BlockSigning.cpp Outdated Show resolved Hide resolved

domob1812 added 4 commits December 3, 2020 14:05

New regtest vaultfork.py.

4c9109a

The new regtest vaultfork.py verifies behaviour around activating the OP_REQUIRE_COINSTAKE fork with an activation time. Once the time has been reached, new blocks will enforce the post-fork rules (i.e. OP_REQUIRE_COINSTAKE and some extra coinstake checks).

domob1812 force-pushed the vault-upgrade branch from eff6bd4 to d93fdc0 Compare December 3, 2020 13:05

galpHub approved these changes Dec 3, 2020

View reviewed changes

galpHub merged commit b287ef3 into DiviProject:Development Dec 3, 2020

domob1812 deleted the vault-upgrade branch December 4, 2020 13:09

domob1812 mentioned this pull request Apr 14, 2021

Custom reward addresses for masternodes #83

Open

Implement fork for "staking vaults" #38

Implement fork for "staking vaults" #38

Uh oh!

Conversation

domob1812 commented Sep 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

galpHub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

galpHub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

domob1812 commented Oct 1, 2020

Uh oh!

galpHub left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

domob1812 commented Oct 23, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

domob1812 commented Nov 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

domob1812 commented Dec 1, 2020

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

domob1812 commented Sep 28, 2020 •

edited

Loading

domob1812 commented Nov 4, 2020 •

edited

Loading