app: 🥞 dynamic application Components

[cratelyn]

penumbra_app::App is a "bundle of Components". it should provide a way to dynamically define its constituent components. this, in tandem with an in-memory consensus engine feeding ABCI messages into the App, would allow us to easily test distinct parts of our state machine.

🏢 App today

as-is, the component stack is hard-coded, in blocks like this:

penumbra/crates/core/app/src/app/mod.rs

Lines 129 to 137 in 59cdd75

	ShieldedPool::init_chain(&mut state_tx, None).await;
	Distributions::init_chain(&mut state_tx, None).await;
	Staking::init_chain(&mut state_tx, None).await;
	Ibc::init_chain(&mut state_tx, None).await;
	Dex::init_chain(&mut state_tx, None).await;
	Governance::init_chain(&mut state_tx, None).await;
	CommunityPool::init_chain(&mut state_tx, None).await;
	Fee::init_chain(&mut state_tx, None).await;
	Funding::init_chain(&mut state_tx, None).await;

penumbra/crates/core/app/src/app/mod.rs

Lines 242 to 256 in 59cdd75

	// Run each of the begin block handlers for each component, in sequence:
	let mut arc_state_tx = Arc::new(state_tx);
	Sct::begin_block(&mut arc_state_tx, begin_block).await;
	ShieldedPool::begin_block(&mut arc_state_tx, begin_block).await;
	Distributions::begin_block(&mut arc_state_tx, begin_block).await;
	Ibc::begin_block::<PenumbraHost, StateDelta<Arc<StateDelta<cnidarium::Snapshot>>>>(
	&mut arc_state_tx,
	begin_block,
	)
	.await;
	CommunityPool::begin_block(&mut arc_state_tx, begin_block).await;
	Governance::begin_block(&mut arc_state_tx, begin_block).await;
	Staking::begin_block(&mut arc_state_tx, begin_block).await;
	Fee::begin_block(&mut arc_state_tx, begin_block).await;
	Funding::begin_block(&mut arc_state_tx, begin_block).await;

penumbra/crates/core/app/src/app/mod.rs

Lines 363 to 372 in 59cdd75

	let mut arc_state_tx = Arc::new(state_tx);
	ShieldedPool::end_block(&mut arc_state_tx, end_block).await;
	Distributions::end_block(&mut arc_state_tx, end_block).await;
	Ibc::end_block(&mut arc_state_tx, end_block).await;
	Dex::end_block(&mut arc_state_tx, end_block).await;
	CommunityPool::end_block(&mut arc_state_tx, end_block).await;
	Governance::end_block(&mut arc_state_tx, end_block).await;
	Staking::end_block(&mut arc_state_tx, end_block).await;
	Fee::end_block(&mut arc_state_tx, end_block).await;
	Funding::end_block(&mut arc_state_tx, end_block).await;

(...and so on...)

invoking these programmatically would help prevent bugs as we continue to maintain and extend App. for example, it's easy to miss that the Sct component is only invoked upon init_chain and begin_block. components should be consistently invoked by the App. (#3739)

🧪 testing benefits

in #3588, we are building an in-memory mock consensus engine. as background, regarding our goals with testing, consider these excerpts from #1664:

[O]ur state machine is written as a series of traits that compose additional behavior onto StateRead/StateWrite to ensure that all execution can be done transactionally[.]

We want to be able to write unit tests that check parts of the state machine in isolation. For instance, we'd like to be able to check individual consensus rules (does this consensus rule reject this input?), or to be able to check pre/post conditions for specific parts of the state machine (do we maintain the desired invariants after processing this input? do we write the data we expect?).

as is, the unit tests in penumbra-app work with a different, specific subset of components, by hard-coding the calls to these hooks, like so:

penumbra/crates/core/app/src/tests/swap_and_swap_claim.rs

Lines 82 to 85 in 59cdd75

	// Execute EndBlock for the Dex, to actually execute the swaps...
	Dex::end_block(&mut state, &end_block).await;
	ShieldedPool::end_block(&mut state, &end_block).await;

in a sense, this is re-implementing the internals of App, which makes tests both more tedious to implement and more brittle to change over time.

an interface to select components programmatically would allow test authors to connect a mock consensus engine to distinct state machine components, and generally write unit tests more effectively.

[hdevalence]

So far this has been a non-goal. Components have to execute in order, and this design assumption cannot be easily changed. Making components dynamic would have to have some way of dynamically specifiying constraints on execution ordering, adding additional complexity.

However, it's a little unclear what the upside would be. Why is it desirable to be able to punch out arbitrary subsets of the Penumbra application? Why don't we just test the whole thing, first?

[cratelyn]

However, it's a little unclear what the upside would be. Why is it desirable to be able to punch out arbitrary subsets of the Penumbra application? Why don't we just test the whole thing, first?

i am a little surprised to hear this! this seemed like a reasonable approach after our conversation in #3695, where you said:

Do we want to write tests of pd? I'm not sure that should be our goal. Our existing app-level integration tests are part of the app crate. What functionality should be in pd as a library rather than in the app crate?

we had a conversation after that where you pointed me towards #1664, which states that "we want to be able to write unit tests that check parts of the state machine in isolation."

our app tests today do this by working directly with components like Dex, CompactBlockManager, and ShieldedPool. but it seemed to me like a better way to do this in the long run would be to create an App containing these components, instead of rewriting parts of the App internals out-of-band in test functions.

[cratelyn]

(following up to the above) ☝️ perhaps an unstated assumption here on my part is that this would not be about changing the application components in the "real" App. those should stay untouched, and the "stack" as-is should be provided as an ergonomic default.

[cratelyn]

@hdevalence talked today and after some direct discussion, i agree this can be put aside for the time being. i'm going to mark this as a low priority and save it for later on in the broader in-memory consensus effort.

[cratelyn]

after further thought, going to close this as unplanned work.