multi: DCP0009 Auto revocations consensus change.

[rstaudt2]

Requires #2719.

---

This introduces automatic ticket revocations as defined in DCP0009. The high-level changes are:

• Allowing tickets to be revoked by anyone instead of requiring a valid signature from the voting address
• Requiring blocks to contain ticket revocation transactions for all tickets that become missed or expired as of that block

Additional details and discussion can also be found in the Politeia Proposal for this change.

---

Consensus rule changes - Overview

The consensus changes introduced to support automatic ticket revocations are as follows:

• Revocation transactions MUST be version 2
• Revocation transaction inputs MUST NOT have a signature script
• Revocation transaction fees MUST be zero
• For revocations output amounts, each remaining atom in the remainder after proportionally distributing ticket redeemer output amounts must be assigned to an output in a deterministic uniformly pseudorandom manner (this rule change was required in order to enforce zero fee for revocations)
• Script validation MUST be skipped for revocation transactions
• Blocks MUST contain revocation transactions for all tickets that will become missed or expired as of that block
• Revocations may spend tickets that will become missed or expired as of that block
  • Previously they could not be spent until the block following the block where they became missed or expired
• Revocations may spend tickets after ticket maturity + 1 blocks (in the block that they become missed or expired)
  • Previously they could spend tickets after ticket maturity + 2 blocks (in the block AFTER they become missed or expired)

For further details see DCP0009.

---

Consensus rule changes - Detailed

This section outlines the changes in commit multi: DCP0009 Auto revocations consensus change., which contains all of the consensus rule changes to support automatic ticket revocations:

• blockchain/scriptval.go
  • ValidateTransactionScripts, checkBlockScripts
    • Skip script validation for version 2 revocations if the automatic ticket revocations agenda is active
• blockchain/stake/staketx.go, blockchain/stake/staketx_test.go
  • Introduce TxVersionAutoRevocations const for version 2 revocations
  • CalculateRewards, CalculateRevocationRewards
    • Add CalculateRevocationRewards to handle the required updates for revocations while leaving CalculateRewards as-is for votes
    • If the automatic ticket revocations agenda is active, and there is a remainder after distributing the returns, then select a uniformly pseudorandom output index to receive each remaining atom
    • Add additional tests to cover the new code paths
  • CheckSSRtx
    • Add checks to ensure that the input does not have a signature script and does not have a fee if the automatic ticket revocations agenda is active
    • Add additional tests to cover the new code paths
• blockchain/validate.go, blockchain/validate_test.go
  • checkTransactionContext
    • Add rule that revocation transactions MUST be version 2 if the automatic ticket revocations agenda is active
  • checkTicketRedeemers
    • Add rule that if the automatic ticket revocations agenda is active, then the block MUST contain revocation transactions for all tickets that will become missed or expired as of that block
    • Add rule that if the automatic ticket revocations agenda is active, then revocations can spend tickets that will become missed or expired as of that block (previously they could not be spent until the block following the block where they became missed or expired)
    • Update to accept the referenced ticket hashes rather than the overall block to simplify adding comprehensive unit tests for the function
  • calcTicketReturnAmounts
    • Update to calculate the amounts for all outputs rather than a single output
    • For revocations, if the automatic ticket revocations agenda is active, and there is a remainder after distributing the returns, then select a uniformly pseudorandom output index to receive each remaining atom
  • checkTicketRedeemerCommitments
    • Add rule that if the transaction is a version 2 revocation and the automatic ticket revocation agenda is active, then the fee MUST be zero
    • Use the updated calcTicketReturnAmounts function so that returns include the full amount, which allows for ensuring a truly zero fee
  • checkRevocationInputs
    • Add rule that if the automatic ticket revocations agenda is active, then revocations can spend tickets after ticket maturity + 1 blocks (in the block that they become missed or expired) rather than after ticket maturity + 2 blocks (in the block AFTER they become missed or expired)
• internal/mempool/mempool.go, internal/mempool/mempool_test.go
  • Pass best block header to CheckTransactionInputs
  • Pass isAutoRevocationsEnabled to ValidateTransactionScripts
• internal/mining/mining.go, internal/mining/mining_harness_test.go,
  internal/mining/mining_test.go, internal/mining/mining_view_test.go
  • Add a helper function, createRevocationFromTicket, to create a revocation transaction from a ticket hash
  • If the automatic ticket revocations agenda is active, then create version 2 revocation transactions for all tickets that will become missed or expired as of the block being created
  • Create and insert these revocations into the priority queue AFTER votes are done being added to ensure that revocations are added for votes that fail validation checks or are otherwise not included into the block
  • Pass best block header to CheckTransactionInputs
  • Pass isAutoRevocationsEnabled to ValidateTransactionScripts
• internal/rpcserver/interface.go, internal/rpcserver/rpcserver.go,
  internal/rpcserver/rpcserverhandlers_test.go
  • handleCreateRawSSRtx
    • If the automatic ticket revocations agenda is active:
      • Validate that the fee is zero
      • Set the transaction version to 2
    • Pass previous header bytes and isAutoRevocationsEnabled flag to CreateRevocationFromTicket in order to create version 2 revocation transactions properly with the updated rules
    • Add additional tests to cover the new code paths
• server.go
  • Pass previous block header to CheckTransactionInputs
  • Pass isAutoRevocationsEnabled to ValidateTransactionScripts

---

Additional Test Coverage

There are several additional commits following the consensus change commit, which add comprehensive tests coverage for the updated consensus rules:

• Add full test coverage for the calcTicketReturnAmounts
• Add full test coverage for the checkTicketRedeemers
• Add validation tests to ensure that all of the validation rules associated with the automatic ticket revocations agenda work as expected
• Add tests to ensure that creating new block templates works as expected when the automatic ticket revocations agenda is enabled
• Add tests to ensure that revocations are accepted and rejected by the mempool as expected with the automatic ticket revocations agenda enabled

---

Impact to External Software

• stake.CalculateRewards has not changed, but should no longer be used for revocations and will return incorrect results for revocations when the automatic ticket revocations agenda is active
• stake.CalculateRevocationRewards has been added should be used for revocations instead of stake.CalculateRewards
• stake.DetermineTxType, stake.IsSSRtx, and stake.CheckSSRtx have been updated to take a isAutoRevocationsEnabled bool param (as described in multi: Add isAutoRevocationsEnabled to CheckSSRtx. #2719)
• createrawssrtx will now return an error if the automatic ticket revocations agenda is active and a non-zero fee is provided

---

Note on the isAutoRevocationsEnabled flag

One additional thing to note about the consensus rule changes is that if the agenda activates, and there are no version 2 revocation transactions in blocks before the agenda activates, then most of the rules can be retroactively updated to be guarded by the revocation transaction version rather than the agenda being active. The guard clauses could be simplified to:

• If the automatic ticket revocations agenda is active, all revocation transactions must be version 2
• All other consensus rules introduced by the automatic ticket revocations agenda would be enforced for version 2 revocation transactions (and would not need to check the agenda)

This is desired because it doesn't require passing around whether the agenda is active, which itself is based on the blockchain global state, and instead could just be based on the transaction itself.

[rstaudt2]

Just added a mining debug log to indicate when automatic revocations are being added to a block in the last push that is helpful for testing.

[rstaudt2]

For reference, in addition to the automated tests, I've also been testing this under various conditions in simnet by doing the following:

• Since all agendas are active by default in simnet, stub isAutoRevocationsAgendaActive to easily simulate the agenda activating at a given height, e.g.:

  func (b *BlockChain) isAutoRevocationsAgendaActive(prevNode *blockNode) (bool, error) {
      return prevNode.height >= 1000, nil
  }

• Simulate missed tickets by modifying the mining code to randomly not include 0-2 votes
• Simulate expired tickets just by mining enough blocks to hit that case
• When the automatic ticket revocations agenda is inactive
  • Validate that the mining code is not generating automatic revocations and that blocks are being accepted as expected as tickets become missed or expired
• When the automatic ticket revocations agenda is active
  • Validate that the mining code is generating automatic revocations as expected and that blocks are getting accepted with the updated consensus rules
  • Perform negative tests by modifying the mining code to disable the automatic revocation generation for missed and expired tickets and validate that the ErrNoMissedTicketRevocation and ErrNoExpiredTicketRevocation errors are returned as expected
  • Use createrawssrtx and sendrawtransaction to create revocations for tickets that were missed and expired prior to the agenda activating and validate that they are accepted into blocks as expected

[davecgh]

* Since all agendas are active by default in `simnet`, ...

What I usually do is just add a temporary definition to the simnet params, modify wallet to vote on the agenda, generate enough blocks to ensure the agenda activates, and then test things.

I haven't done that with this PR yet, but I plan to this weekend.

[JoeGruffins]

Trying this out on simnet looks to work as intended, but mining blocks that include automatic revocations takes some time. There is tangible lag compared to blocks without revocations. Maybe just how it is?

[JoeGruffins]

I attempted to graph what what I see. My mining script looks like:

#!/usr/bin/env bash
NUM=1
  case $1 in
      ''|*[!0-9]*)  ;;
      *) NUM=$1 ;;
  esac
  for i in $(seq $NUM) ; do
    dcrctl -C /home/joe/dextest/dcr/alpha/alpha-ctl.conf regentemplate
    sleep 0.05
    dcrctl -C /home/joe/dextest/dcr/alpha/alpha-ctl.conf generate 1
    if [  !=  ]; then
      sleep 0.5
    fi
  done

And what I could get from psrecord a python program with three different one ticket auto revocations:

graphs

The zero pc usage at the beginning and start is before and after mining some blocks, I think it was 20,30,30 blocks respectively for these graphs. You can see where it looks like from here that dcrd just pauses for a while when the revocations take place. Perhaps something with my setup? Using this https://github.com/decred/dcrdex/tree/master/dex/testing/dcr

[rstaudt2]

Trying this out on simnet looks to work as intended, but mining blocks that include automatic revocations takes some time. There is tangible lag compared to blocks without revocations. Maybe just how it is?

I'm not seeing a delay or any significant difference in the time to generate blocks with auto revocations. Did you add any test code to miss votes or how are you handling that? Perhaps that is causing the issue or would otherwise help me reproduce this.

[JoeGruffins]

Did you add any test code to miss votes or how are you handling that?

Not handling... the tickets being revoked belong to wallets that aren't part of purchasing tickets needed to sustain block propagation. I'm just doing a very mechanical, purchase ticket with wallet and shut it down, progress the chain.

Here is a log where you can see a couple seconds "pause":

2021-09-07 11:17:02.920 [DBG] PEER: Sending tx (hash f5e614f3e302e99f840aa41c21bbfb914280220adceabcf6f6671eaaba8b7274, 1 inputs, 3 outputs, lock height 0) to 127.0.0.1:60026 (inbound)                                                                                         
2021-09-07 11:17:03.077 [DBG] RPCS: Received command <regentemplate> from 127.0.0.1:38956                                                                                                                                                                                       
2021-09-07 11:17:03.137 [DBG] RPCS: Received command <generate> from 127.0.0.1:38956                                                                                                                                                                                            
2021-09-07 11:17:03.420 [DBG] PEER: Received inv (txns 11, blocks 0, other 0) from 127.0.0.1:60026 (inbound)                                                                                                                                                                    
2021-09-07 11:17:05.081 [DBG] MINR: Found eligible parent 5b92f55ddc3f6087e4021c35c74ce55af8477be7b0d1e460f1758afed5bc0d8f with enough votes to build block on, proceeding to create a new block template                                                                       
2021-09-07 11:17:05.081 [DBG] MINR: Considering 11 transactions for inclusion to new block                                                                                                                                                                                      
2021-09-07 11:17:05.081 [DBG] MINR: Generated 1 automatic revocations for the new block (num missed this block: 1, num expired this block: 0)                                                                                                                                   
2021-09-07 11:17:05.083 [DBG] MINR: Created new block template (2 transactions, 12 stake transactions, 0 treasury transactions, 17768 in fees, 25 signature operations, 4025 bytes, target difficulty 7fffff0000000000000000000000000000000000000000000000000000000000, stake di
fficulty 0.00309595)                                                                                                                                                                                                                                                            
2021-09-07 11:17:05.083 [DBG] PEER: Sending headers (num 1, final hash 37f9cc9f773d88bcd1bfba908d550355db56a23fa1cd755f1e8c607fe1fac16e, height 625) to 127.0.0.1:60026 (inbound)                                                                                               
2021-09-07 11:17:05.084 [DBG] PEER: Received getdata (block 37f9cc9f773d88bcd1bfba908d550355db56a23fa1cd755f1e8c607fe1fac16e) from 127.0.0.1:60026 (inbound)                                                                                                                    
2021-09-07 11:17:05.084 [DBG] PEER: Misbehaving whitelisted peer 127.0.0.1:60026 (inbound): getdata                                                                                                                                                                             
2021-09-07 11:17:05.084 [DBG] PEER: Sending block (hash 37f9cc9f773d88bcd1bfba908d550355db56a23fa1cd755f1e8c607fe1fac16e, ver 8, 2 tx, 2021-09-07 11:17:05 +0900 JST) to 127.0.0.1:60026 (inbound)                                                                              
2021-09-07 11:17:05.085 [DBG] PEER: Received headers (num 1, final hash 37f9cc9f773d88bcd1bfba908d550355db56a23fa1cd755f1e8c607fe1fac16e, height 625) from 127.0.0.1:60026 (inbound)                                                                                            
2021-09-07 11:17:05.096 [DBG] FEES: Updated moving averages into block 625

Another one:

2021-09-07 11:55:29.752 [DBG] PEER: Sending tx (hash 32f15629ff88c4eb8c82e96531644e6ec299066be198a7a31eefcfe90767c3b2, 1 inputs, 3 outputs, lock height 0) to 127.0.0.1:60032 (inbound)
2021-09-07 11:55:29.752 [DBG] PEER: Sending tx (hash c9fd9aa20ebc977651d49c80924f6f4f3c19f4d2030fac52c2503b41086e9429, 1 inputs, 3 outputs, lock height 0) to 127.0.0.1:60032 (inbound)
2021-09-07 11:55:30.138 [DBG] RPCS: Received command <regentemplate> from 127.0.0.1:38964
2021-09-07 11:55:30.198 [DBG] RPCS: Received command <generate> from 127.0.0.1:38964                     
2021-09-07 11:55:30.252 [DBG] PEER: Received inv (txns 11, blocks 0, other 0) from 127.0.0.1:60032 (inbound)
2021-09-07 11:55:32.152 [DBG] MINR: Found eligible parent 6e9a58d9f310d4da09f20658df17559112a638bc9c5dd3c13868f4ee3383e661 with enough votes to build block on, proceeding to create a new block template
2021-09-07 11:55:32.152 [DBG] MINR: Considering 11 transactions for inclusion to new block         
2021-09-07 11:55:32.152 [DBG] MINR: Generated 1 automatic revocations for the new block (num missed this block: 1, num expired this block: 0)                                                                                                                                   
2021-09-07 11:55:32.153 [DBG] MINR: Created new block template (2 transactions, 12 stake transactions, 0 treasury transactions, 17768 in fees, 25 signature operations, 4023 bytes, target difficulty 7888870000000000000000000000000000000000000000000000000000000000, stake di
fficulty 0.0002)                                                                                                                                                                                                                                                                
2021-09-07 11:55:32.154 [DBG] PEER: Sending headers (num 1, final hash 5e86315fd527d978f5768d63592f8d9de1d5a5a25cd94e97fabd9c97e6c74d2d, height 764) to 127.0.0.1:60032 (inbound)

And what it normally looks like:

2021-09-07 11:17:02.420 [DBG] PEER: Sending tx (hash 6fd2b888d2fcd674a8242b671b8de0a4e703108b3b5527bd26c1636f426930cf, 1 inputs, 3 outputs, lock height 0) to 127.0.0.1:60026 (inbound)                                                                                         
2021-09-07 11:17:02.420 [DBG] PEER: Sending tx (hash 1c8588c8decaf5c54cadf5384936ec81b7d7f98cecd0fc0414d74ab2fe1ca8e1, 1 inputs, 3 outputs, lock height 0) to 127.0.0.1:60026 (inbound)                                                                                         
2021-09-07 11:17:02.495 [DBG] RPCS: Received command <regentemplate> from 127.0.0.1:38956                                                                                                                                                                                       
2021-09-07 11:17:02.495 [DBG] MINR: Found eligible parent 52104a98e9a2f84a2f7045194fecc313b9d3ed2fb0b6a7ef3602c3a516d80db5 with enough votes to build block on, proceeding to create a new block template                                                                       
2021-09-07 11:17:02.498 [DBG] MINR: Considering 6 transactions for inclusion to new block                                                                                                                                                                                       
2021-09-07 11:17:02.502 [DBG] CHAN: Difficulty retarget at block height 624                                                                                                                                                                                                     
2021-09-07 11:17:02.502 [DBG] CHAN: Old target 207fffff (7fffff0000000000000000000000000000000000000000000000000000000000)                                                                                                                                                      
2021-09-07 11:17:02.502 [DBG] CHAN: New target 207fffff (7fffff0000000000000000000000000000000000000000000000000000000000)                                                                                                                                                      
2021-09-07 11:17:02.502 [DBG] CHAN: Difficulty retarget at block height 624                                                                                                                                                                                                     
2021-09-07 11:17:02.502 [DBG] CHAN: Old target 207fffff (7fffff0000000000000000000000000000000000000000000000000000000000)                                                                                                                                                      
2021-09-07 11:17:02.502 [DBG] CHAN: New target 207fffff (7fffff0000000000000000000000000000000000000000000000000000000000)                                                                                                                                                      
2021-09-07 11:17:02.503 [DBG] MINR: Created new block template (2 transactions, 6 stake transactions, 0 treasury transactions, 4330 in fees, 13 signature operations, 2477 bytes, target difficulty 7fffff0000000000000000000000000000000000000000000000000000000000, stake diff
iculty 0.00309595)                                                                                                                                                                                                                                                              
2021-09-07 11:17:02.555 [DBG] RPCS: Received command <generate> from 127.0.0.1:38956                                                                                                                                                                                            
2021-09-07 11:17:02.555 [DBG] CHAN: Difficulty retarget at block height 624                                                                                                                                                                                                     
2021-09-07 11:17:02.555 [DBG] CHAN: Old target 207fffff (7fffff0000000000000000000000000000000000000000000000000000000000)                                                                                                                                                      
2021-09-07 11:17:02.555 [DBG] CHAN: New target 207fffff (7fffff0000000000000000000000000000000000000000000000000000000000)  

So I see a pause between Received inv ... and Found eligible parent ... when auto-revoking.

[JoeGruffins]

Ok! It seems this is the same behavior as master! So... false alarm. Sorry about that. There is always a pause when revocations are present, perhaps something with my setup if you do not see them though.

[rstaudt2]

Ok! It seems this is the same behavior as master! So... false alarm. Sorry about that. There is always a pause when revocations are present, perhaps something with my setup if you do not see them though.

Okay, great! Thanks for the review and for confirming. I'm not sure where that pause is occurring on your side without more detailed logs, but given that the delay is right after Received inv..., and the CPU of the graphs you shared previously is at zero during that time, it seems likely that it may be blocked on a network call. It does not appear related to any of the changes in NewBlockTemplate since the Found eligible parent... log is right at the beginning of that function.

[davecgh]

This looks great overall thus far. The commits are well crafted, the core logic all appears correct, and the tests provide good coverage.

I still have some more review of the tests themselves to double check they're all testing what they claim to test fully and some manual testing to do, but I have reviewed the core validation logic carefully and wanted to go ahead and get the first part of the review out.

[davecgh]

I have gone through all of the tests to verify that are indeed testing what they claim. They look great overall. Just a few minor nits I noticed will going through that I've commented on inline.

[matheusd]

Tested revoking with the existing semantics and the new ones and revoking a large number of tickets after moving to the new semantics.

Looking very tight!

Will take a second look after the updates you mentioned in the last inline comment.

[davecgh]

The update to remove revocations from the mempool on block changes once the agenda is active look correct and I also tested it works as intended on simnet by modifying the code a bit to get it into the state where it applies.

I should also note that technically speaking a revocation only really needs to be removed if it is no longer valid as opposed to just always removing it since, most of the time, it will still be valid given there is typically only a single output. However, given this case will only ever apply to revocations for a finite set of legacy tickets that are going to be manually revoked, unconditionally removing them is reasonable.

[matheusd]

Tested the last changes working as expected.

Another note re: always dropping the revocation is that this might make it slightly annoying to issue the revocations for the old tickets, since it means users will have to actively monitor (and potentially keep issuing the revocation) until it's successfully included in a block. Not a show stopper since it's just a temporary issue.

LGTM. Excellent work!

[rstaudt2]

Another note re: always dropping the revocation is that this might make it slightly annoying to issue the revocations for the old tickets, since it means users will have to actively monitor (and potentially keep issuing the revocation) until it's successfully included in a block. Not a show stopper since it's just a temporary issue.

Agreed, that is a good point that @davecgh raised as well. I'll be writing the script to issue the revocations for the legacy tickets, which will take this into account (so no one else should have to deal with it). Thanks for the review!