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

tests: ⭐ mock consensus can InitChain with a single validator #3816

Closed
cratelyn opened this issue Feb 13, 2024 · 1 comment · Fixed by #3902
Closed

tests: ⭐ mock consensus can InitChain with a single validator #3816

cratelyn opened this issue Feb 13, 2024 · 1 comment · Fixed by #3902
Assignees
Labels
A-mock-consensus Area: Relates to the mock consensus engine C-enhancement Category: an enhancement to the codebase _P-high High priority
Milestone

Comments

@cratelyn
Copy link
Contributor

see #3588.

with #3787 complete, we can proceed to the next phase of work on the mock consensus testing framework: we should implement the logic to patch an AppState with the information for a single test validator.

see this todo comment:

// what to do here?
// - read out list of abci/comet validators from the builder,
// - define a penumbra validator for each one
// - inject that into the penumbra app state
// - serialize to json and then call `with_app_state_bytes`

@cratelyn cratelyn added C-enhancement Category: an enhancement to the codebase A-mock-consensus Area: Relates to the mock consensus engine labels Feb 13, 2024
@github-project-automation github-project-automation bot moved this to 🗄️ Backlog in Penumbra Feb 13, 2024
@cratelyn cratelyn added the _P-high High priority label Feb 13, 2024
@cratelyn cratelyn self-assigned this Feb 13, 2024
@cratelyn cratelyn changed the title tests: ⭐ mock consensus can define a single validator tests: ⭐ mock consensus can InitChain with a single validator Feb 13, 2024
@aubrika aubrika added this to the Sprint 0 milestone Feb 13, 2024
@aubrika aubrika moved this from 🗄️ Backlog to Todo in Penumbra Feb 13, 2024
@hdevalence
Copy link
Member

One note: it's actually possible this might not be a blocker relative to block production.

The reason we need to modify the genesis data to match the mock engine's validators (namely, defining corresponding penumbra validators and allocating delegation tokens at genesis) is that otherwise the staking component will have no validators, feed that back to Comet as an update, and Comet will halt rather than setting the validator set to the empty set.

But currently the mock engine doesn't do any validator set tracking anyways. So it seems possible that as long as we're not trying to test the staking component specifically, this isn't a blocker.

@cratelyn cratelyn moved this from Todo to In progress in Penumbra Feb 27, 2024
@cratelyn cratelyn modified the milestones: Sprint 0, Sprint 1 Feb 27, 2024
cratelyn added a commit that referenced this issue Feb 29, 2024
fixes #3816.

this provides extension facilities to the mock consensus test node
builder, which allows penumbra-app tests to define a single validator,
and subsequently retrieve it from the chain state.

see `mock_consensus_can_define_a_genesis_validator`. when run with
`--no-capture` enabled, these logs will be visible:

```
  2024-02-28T23:00:43.751036Z DEBUG penumbra_stake::component::stake: adding validator identity to consensus set index, validator: penumbravalid172v76yyqwngcln2dxrs8ht0sjgswer3569yyhezgsz6aj97ecvqqyf3h9h
    at crates/core/component/stake/src/component/stake.rs:533
    in penumbra_stake::component::stake::staking

[...]

  2024-02-28T23:00:43.776880Z  INFO penumbra_app::server::consensus: genesis state is a full configuration
    at crates/core/app/src/server/consensus.rs:145

  2024-02-28T23:00:43.780436Z DEBUG penumbra_app::app: finished committing state, jmt_root: RootHash("46dc0e9561f17eee61a2c13f517036d4d0a4c77c60362cb6cc165083675dcaf7")
    at crates/core/app/src/app/mod.rs:592
```

logging facilities are provided so that helper warnings should be given
to users that forget to call `with_penumbra_single_validator`, or
provide an `AppState` object whose validator list would be overwritten.

the `serde_json` dependency is removed from the mock consensus library,
it is no longer used.

a warning is added to the mock consensus library to note to future
contributors that other penumbra dependencies should be avoided in that
library.

a new `http::Extensions` field is added to the builder. it is not used
internally, but provides extension traits a place to hold additional
state.

* #3588
* #3816
cratelyn added a commit that referenced this issue Mar 1, 2024
fixes #3816.

this provides extension facilities to the mock consensus test node
builder, which allows penumbra-app tests to define a single validator,
and subsequently retrieve it from the chain state.

see `mock_consensus_can_define_a_genesis_validator`. when run with
`--no-capture` enabled, these logs will be visible:

```
  2024-02-28T23:00:43.751036Z DEBUG penumbra_stake::component::stake: adding validator identity to consensus set index, validator: penumbravalid172v76yyqwngcln2dxrs8ht0sjgswer3569yyhezgsz6aj97ecvqqyf3h9h
    at crates/core/component/stake/src/component/stake.rs:533
    in penumbra_stake::component::stake::staking

[...]

  2024-02-28T23:00:43.776880Z  INFO penumbra_app::server::consensus: genesis state is a full configuration
    at crates/core/app/src/server/consensus.rs:145

  2024-02-28T23:00:43.780436Z DEBUG penumbra_app::app: finished committing state, jmt_root: RootHash("46dc0e9561f17eee61a2c13f517036d4d0a4c77c60362cb6cc165083675dcaf7")
    at crates/core/app/src/app/mod.rs:592
```

logging facilities are provided so that helper warnings should be given
to users that forget to call `with_penumbra_single_validator`, or
provide an `AppState` object whose validator list would be overwritten.

the `serde_json` dependency is removed from the mock consensus library,
it is no longer used.

a warning is added to the mock consensus library to note to future
contributors that other penumbra dependencies should be avoided in that
library.

a new `http::Extensions` field is added to the builder. it is not used
internally, but provides extension traits a place to hold additional
state.

* #3588
* #3816
cratelyn added a commit that referenced this issue Mar 7, 2024
fixes #3816. see #3588. this also addresses part of #3934.

this provides extension facilities to the mock consensus test node
builder, which allows penumbra-app tests to define a single validator,
and subsequently retrieve it from the chain state.

see `mock_consensus_can_define_a_genesis_validator`. when run with
`--no-capture` enabled, these logs will be visible:

```
2024-02-28T23:00:43.751036Z DEBUG penumbra_stake::component::stake: adding validator identity to consensus set index, validator: penumbravalid172v76yyqwngcln2dxrs8ht0sjgswer3569yyhezgsz6aj97ecvqqyf3h9h
at crates/core/component/stake/src/component/stake.rs:533
in penumbra_stake::component::stake::staking

[...]

2024-02-28T23:00:43.776880Z  INFO penumbra_app::server::consensus: genesis state is a full configuration
at crates/core/app/src/server/consensus.rs:145

2024-02-28T23:00:43.780436Z DEBUG penumbra_app::app: finished committing state, jmt_root: RootHash("46dc0e9561f17eee61a2c13f517036d4d0a4c77c60362cb6cc165083675dcaf7")
at crates/core/app/src/app/mod.rs:592
```

logging facilities are provided so that helper warnings should be given
to users that forget to call `with_penumbra_single_validator`, or
provide an `AppState` object whose validator list would be overwritten.

the `serde_json` dependency is removed from the mock consensus library,
it is no longer used.

a warning is added to the mock consensus library to note to future
contributors that other penumbra dependencies should be avoided in that
library.

* #3588
* #3816

---------

Co-authored-by: Henry de Valence <hdevalence@penumbralabs.xyz>
@github-project-automation github-project-automation bot moved this from In progress to Done in Penumbra Mar 7, 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 C-enhancement Category: an enhancement to the codebase _P-high High priority
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants