Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[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 #2507

Open
jcnelson opened this issue Mar 10, 2021 · 2 comments
Assignees
Labels
consensus-critical icebox Issues that are not being worked on

Comments

@jcnelson
Copy link
Member

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.

@stale
Copy link

stale bot commented Sep 7, 2021

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.

@MaksimalistT
Copy link

MaksimalistT commented Sep 21, 2021

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?

@jcnelson jcnelson removed this from To do in Stacks 2.1 May 23, 2022
@jcnelson jcnelson added the icebox Issues that are not being worked on label Mar 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus-critical icebox Issues that are not being worked on
Projects
Status: Status: 🆕 New
Development

No branches or pull requests

4 participants