mock-consensus: 🌱 single genesis validator #3902

cratelyn · 2024-02-27T19:22:50Z

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.

crates/test/mock-consensus/Cargo.toml

crates/core/app/tests/common/mod.rs

crates/core/app/tests/mock_consensus.rs

crates/test/mock-consensus/src/builder.rs

crates/core/app/tests/mock_consensus.rs

crates/core/app/tests/common/mod.rs

hdevalence · 2024-02-29T18:03:08Z

I think we should take the opportunity to iterate on the API a little bit and figure out how we want to manage validator keys. It might be good to do that in chat or a slightly more synchronous setting than GH issues.

cherry-picked from #3902.

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

for context, see: - #3902 (comment) - #3902 (comment) Co-authored-by: Henry de Valence <[email protected]>

crates/test/mock-consensus/src/builder.rs

crates/core/app/tests/common/test_node_builder_ext.rs

crates/test/mock-consensus/Cargo.toml

crates/test/mock-consensus/src/keyring.rs

crates/core/app/tests/common/test_node_builder_ext.rs

hdevalence · 2024-03-05T23:15:06Z

crates/core/app/tests/mock_consensus.rs

+    // Install a test logger, acquire some temporary storage, and start the test node.
+    let guard = common::set_tracing_subscriber();
+    let storage = TempStorage::new().await?;
+    let _test_node = common::start_test_node(&storage).await?;


We should avoid doing this, so that it's possible to reason about each test in isolation. If the initialization code is so complicated we can't copy paste it between tests we should try to simplify it so that it can be.

crates/test/mock-consensus/src/keyring.rs

crates/test/mock-consensus/src/builder.rs

crates/core/app/tests/common/test_node_builder_ext.rs

#3902 (comment)

https://github.com/penumbra-zone/penumbra/pull/3902/files#r1513611082 > This isn't a valid key, which may cause problems. > > We should instead generate a random signing key, get its verification > key, use that for the identity key, and throw the signing key away > after. a stub is intentionally used here until we have test coverage exercising this. this /will/ cause problems, but i explicitly want to know when! i want to see how far we get before we run into this being an issue.

this addresses a few assorted threads from review: https://github.com/penumbra-zone/penumbra/pull/3902/files#r1513620298 https://github.com/penumbra-zone/penumbra/pull/3902/files#r1513615058 https://github.com/penumbra-zone/penumbra/pull/3902/files#r1513613617 https://github.com/penumbra-zone/penumbra/pull/3902/files#r1513617136 see also, #3934. this replaces the `Keys` structure, and defines a new `Keyring` type. this is added to the builder and used by the `with_penumbra_auto_app_state` extension, but it not yet introduced to `TestNode<C>`.

cratelyn

💬 added some commits to address review comments. most importantly, Keys is replaced with a Keyring. in doing so, we also have addressed (most of) #3934!

crates/core/app/tests/common/test_node_builder_ext.rs

crates/test/mock-consensus/src/builder.rs

crates/test/mock-consensus/src/keyring.rs

cratelyn added C-enhancement Category: an enhancement to the codebase A-mock-consensus Area: Relates to the mock consensus engine labels Feb 27, 2024

cratelyn added this to the Sprint 1 milestone Feb 27, 2024

cratelyn self-assigned this Feb 27, 2024

cratelyn temporarily deployed to smoke-test February 27, 2024 19:22 — with GitHub Actions Inactive

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from 5ad993b to 69b548a Compare February 28, 2024 13:33

cratelyn temporarily deployed to smoke-test February 28, 2024 13:33 — with GitHub Actions Inactive

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from 69b548a to 36f7604 Compare February 28, 2024 18:27

cratelyn temporarily deployed to smoke-test February 28, 2024 18:27 — with GitHub Actions Inactive

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from 36f7604 to 920c3c9 Compare February 28, 2024 18:56

cratelyn had a problem deploying to smoke-test February 28, 2024 18:56 — with GitHub Actions Error

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from 920c3c9 to 23a330b Compare February 28, 2024 19:04

cratelyn temporarily deployed to smoke-test February 28, 2024 19:04 — with GitHub Actions Inactive

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from 23a330b to d17c790 Compare February 28, 2024 22:10

cratelyn temporarily deployed to smoke-test February 28, 2024 22:10 — with GitHub Actions Inactive

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from d17c790 to 65e83fc Compare February 28, 2024 22:59

cratelyn temporarily deployed to smoke-test February 28, 2024 22:59 — with GitHub Actions Inactive

cratelyn commented Feb 28, 2024

View reviewed changes

crates/test/mock-consensus/Cargo.toml Outdated Show resolved Hide resolved

cratelyn commented Feb 28, 2024

View reviewed changes

crates/core/app/tests/common/mod.rs Outdated Show resolved Hide resolved

cratelyn commented Feb 28, 2024

View reviewed changes

crates/core/app/tests/mock_consensus.rs Outdated Show resolved Hide resolved

cratelyn commented Feb 28, 2024

View reviewed changes

crates/test/mock-consensus/src/builder.rs Outdated Show resolved Hide resolved

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from 65e83fc to c40e350 Compare February 29, 2024 17:13

cratelyn had a problem deploying to smoke-test February 29, 2024 17:13 — with GitHub Actions Error

cratelyn commented Feb 29, 2024

View reviewed changes

crates/core/app/tests/mock_consensus.rs Outdated Show resolved Hide resolved

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from c40e350 to 3499c2d Compare February 29, 2024 17:24

cratelyn temporarily deployed to smoke-test February 29, 2024 17:24 — with GitHub Actions Inactive

hdevalence reviewed Feb 29, 2024

View reviewed changes

crates/core/app/tests/common/mod.rs Outdated Show resolved Hide resolved

hdevalence reviewed Feb 29, 2024

View reviewed changes

crates/core/app/tests/common/mod.rs Outdated Show resolved Hide resolved

cratelyn changed the title ~~mock-consensus: 🌱 single genesis validator (work-in-progress)~~ mock-consensus: 🌱 single genesis validator Feb 29, 2024

cratelyn added a commit that referenced this pull request Mar 1, 2024

staking: 🔊 log genesis validators after parsing (#3916)

fc6e9d2

cherry-picked from #3902.

cratelyn mentioned this pull request Mar 1, 2024

keys: 🍴 SpendKeyBytes: From<[u8; SPENDKEY_LEN_BYTES]> #3922

Merged

cratelyn and others added 3 commits March 1, 2024 12:12

app: 🎴 add small doc comment to App::new

a4464f1

mock-consensus: 🔍 rework validator generation (review)

3e8691d

for context, see: - #3902 (comment) - #3902 (comment) Co-authored-by: Henry de Valence <[email protected]>

cratelyn force-pushed the kate/mock-consensus-genesis-validator branch from 3499c2d to 3e8691d Compare March 1, 2024 21:11

cratelyn had a problem deploying to smoke-test March 1, 2024 21:11 — with GitHub Actions Error

mock-consensus: 👋 remove frivolous dependencies

3bd72e8

cratelyn temporarily deployed to smoke-test March 1, 2024 21:13 — with GitHub Actions Inactive