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

staking(tests): add validator tombstoning scenarios #4149

Open
erwanor opened this issue Apr 2, 2024 · 2 comments
Open

staking(tests): add validator tombstoning scenarios #4149

erwanor opened this issue Apr 2, 2024 · 2 comments
Assignees
Labels
A-mock-consensus Area: Relates to the mock consensus engine A-staking Area: Design and implementation of staking and delegation A-testing Area: Relates to testing of Penumbra

Comments

@erwanor
Copy link
Member

erwanor commented Apr 2, 2024

This is perhaps best to split into multiple tickets, but at a high-level we want to:

  • Enshrine in tests that Tombstoned is a terminal state
  • Check that exchange rates and validator rewards are always penalized
  • Check that new delegations are impossible
  • Check that that undelegation claims are instantaneous
  • Check that claims are always factoring in the penalty even if tombstoning happened after undelegating

For context, the application receives misbehavior evidence from comet in the BeginBlock packet (and starting in 0.38, FinalizeBlock either way we want to process evidence before tx execution), the application then forwards it to the staking component in ValidatorManager::process_evidence which then records the appropriate penalty and trigger a state transition.

/// Evidence of validator misbehavior.
///
/// [ABCI documentation](https://docs.tendermint.com/master/spec/abci/abci.html#evidence)
#[derive(Clone, PartialEq, Eq, Debug)]
pub struct Misbehavior {
    /// The kind of evidence.
    ///
    /// Note: this field is called `type` in the protobuf, but we call it `kind`
    /// to avoid the Rust keyword.
    pub kind: MisbehaviorKind,
    /// The offending validator.
    pub validator: Validator,
    /// The height when the offense occurred.
    pub height: block::Height,
    /// The corresponding time when the offense occurred.
    pub time: Time,
    /// Total voting power of the validator set at `height`.
    ///
    /// This is included in case the ABCI application does not store historical
    /// validators, cf.
    /// [#4581](https://github.com/tendermint/tendermint/issues/4581)
    pub total_voting_power: vote::Power,
}

Ostensibly, there is very little validation done by the application because we assume that CometBFT has done it for us. We can't however assume that the evidence gets only delivered once or make any assumptions that are downstream of validator behavior (since it is byzantine!).

This is good news, because this means that to test this part of the staking logic we just need to:

  • have a method that creates a stub Misbehavior datum that specifies some cometbft address (that's the only data we need to populate!)
  • have a hook that inject Misbehavior into a BeginBlock packet

And then we "just" need to inspect the staking state to assert the various invariants that we are looking for.

Originally posted by @erwanor in #4049 (comment)

@github-project-automation github-project-automation bot moved this to Backlog in Penumbra Apr 2, 2024
@erwanor erwanor added A-staking Area: Design and implementation of staking and delegation _P-V1 Priority: slated for V1 release A-testing Area: Relates to testing of Penumbra A-mock-consensus Area: Relates to the mock consensus engine labels Apr 2, 2024
@erwanor
Copy link
Member Author

erwanor commented Apr 3, 2024

@cratelyn pointed out that the preferred pattern is to build mock data in the test and use an interface to inject it in block execution (unless I misunderstood)

@cratelyn
Copy link
Contributor

cratelyn commented Apr 3, 2024

🔗 some handy links

@erwanor, following up with some links to help you on your way!

/// Sets the evidence [`List`][evidence::List] for this block.
pub fn with_evidence(self, evidence: evidence::List) -> Self {
Self { evidence, ..self }
}

the block builder has a with_evidence() method to set a block's list of malfeasance, in its entirety. we may want an add_evidence() method too, like the add_tx, which could append a new piece of evidence, without discarding any existing data.

the evidence field here should no longer be ignored. now, it should be passed along from the block constructed by Builder::finish() as a new parameter to the abci submodule's TestNode::begin_block() method, and used to fill byzantine_validators, here:

byzantine_validators: Default::default(),

i suspect that most of what you'll need is already there! the one wrinkle will be sorting out the discrepancy between the tendermint::evidence::Evidence stored in a block's evidence: List, and the begin block request's byzantine_validators. it looks like there's a TryFrom<Evidence> we can use for that.

@erwanor erwanor self-assigned this Apr 3, 2024
@aubrika aubrika added this to the Sprint 4 milestone Apr 8, 2024
@aubrika aubrika moved this from Backlog to Todo in Penumbra Apr 8, 2024
@cratelyn cratelyn modified the milestones: Sprint 4, Sprint 5 Apr 22, 2024
@aubrika aubrika added the friction something made this fall into the following milestone & the reason should be noted in a comment label May 6, 2024
@aubrika aubrika modified the milestones: Sprint 5, Sprint 6 May 6, 2024
@cratelyn cratelyn removed the friction something made this fall into the following milestone & the reason should be noted in a comment label May 6, 2024
@aubrika aubrika removed the _P-V1 Priority: slated for V1 release label May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-mock-consensus Area: Relates to the mock consensus engine A-staking Area: Design and implementation of staking and delegation A-testing Area: Relates to testing of Penumbra
Projects
Status: Todo
Development

No branches or pull requests

3 participants