stacks-core icon indicating copy to clipboard operation
stacks-core copied to clipboard

[burnchain] the block-commit at the end of the reward cycle's pay-out period will never be valid if the size of the reward set is odd

Open jcnelson opened this issue 3 years ago • 2 comments

The LeaderBlockCommitOp::check_pox() method contains a snippet of code that will ensure that a reward cycle with an odd number of reward slots is terminated with a burn address, thereby ensuring that all PoX block-commits pay to two addresses if they pay out anything at all. Specifically, the code that does this padding is:

let mut check_recipients: Vec<_> = reward_set_info
   .recipients
   .iter()
   .map(|(addr, _)| addr.clone())
   .collect();

if check_recipients.len() == 1 {
   // If the number of recipients in the set was even, we need to pad
   // with a burn address
   check_recipients
      .push(StacksAddress::burn_address(burnchain.is_mainnet()))
}

(Pretty sure that comment about being "even" is a documentation bug -- the only way check_recipients.len() == 1 could ever be true is if the reward set size was odd).

The method Burnchain::is_mainnet() is buggy -- it always returns false because it currently compares the Bitcoin network ID to the Stacks network ID to determine if the node is running on mainnet. As such, check_recipients gets padded with a testnet burn address. This, in turn, causes the validation logic to always fail on mainnet, because it determines that the block-commit's burn address is somehow different from the testnet burn address. You can see it in the logs; for example:

WARN [1615401483.330144] [src/chainstate/burn/operations/leader_block_commit.rs:594] [chains-coordinator] Invalid block commit: committed output ST000000000000000000002AMW42H does not match expected SP000000000000000000002Q6VF78

The bad news is that fixing Burnchain::is_mainnet() would be a consensus-breaking change -- it would render previously-invalid block-commits valid. We'd need to address this in Stacks 2.1. The good news is that Burnchain::is_mainnet() is only used here, and this bug only manifests under very specific circumstances that aren't really exploitable. So, while this is a consensus bug, it has a small "blast radius."

In the mean time, miners will likely just want to avoid mining in the last sortition of the payout period of a reward cycle -- their blocks won't be valid, and they'll miss the sortition no matter what they do.

jcnelson avatar Mar 10 '21 21:03 jcnelson

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Sep 07 '21 03:09 stale[bot]

In the mean time, miners will likely just want to avoid mining in the last sortition of the payout period of a reward cycle -- their blocks won't be valid, and they'll miss the sortition no matter what they do.

@jcnelson is there a way to determine last sortition from logs? is there a way to avoid current block?

MaksimalistT avatar Sep 21 '21 17:09 MaksimalistT