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

app: 💸 a mock consensus spend is performed #3891

Merged
merged 4 commits into from
Feb 27, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 22 additions & 5 deletions crates/core/app/src/action_handler/transaction/stateful.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ pub async fn fmd_parameters_valid<S: StateRead>(state: S, transaction: &Transact
)
}

#[tracing::instrument(
skip_all,
fields(
current_fmd.precision_bits = current_fmd_parameters.precision_bits,
previous_fmd.precision_bits = previous_fmd_parameters.precision_bits,
previous_fmd.as_of_block_height = previous_fmd_parameters.as_of_block_height,
block_height,
)
)]
pub fn fmd_precision_within_grace_period(
tx: &Transaction,
previous_fmd_parameters: fmd::Parameters,
Expand All @@ -42,13 +51,21 @@ pub fn fmd_precision_within_grace_period(
{
// Clue must be using the current `fmd::Parameters`, or be within
// `FMD_GRACE_PERIOD_BLOCKS` of the previous `fmd::Parameters`.
if clue.precision_bits() == current_fmd_parameters.precision_bits
|| (clue.precision_bits() == previous_fmd_parameters.precision_bits
&& block_height
< previous_fmd_parameters.as_of_block_height + FMD_GRACE_PERIOD_BLOCKS)
{
let clue_precision = clue.precision_bits();
let using_current_precision = clue_precision == current_fmd_parameters.precision_bits;
let using_previous_precision = clue_precision == previous_fmd_parameters.precision_bits;
let within_grace_period =
block_height < previous_fmd_parameters.as_of_block_height + FMD_GRACE_PERIOD_BLOCKS;
if using_current_precision || (using_previous_precision && within_grace_period) {
continue;
} else {
tracing::error!(
%clue_precision,
%using_current_precision,
%using_previous_precision,
%within_grace_period,
"invalid clue precision"
);
anyhow::bail!("consensus rule violated: invalid clue precision");
}
}
Expand Down
89 changes: 66 additions & 23 deletions crates/core/app/tests/mock_consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,22 @@ mod common;
use {
anyhow::anyhow,
cnidarium::TempStorage,
penumbra_fee::Fee,
penumbra_keys::test_keys,
penumbra_mock_client::MockClient,
penumbra_mock_consensus::TestNode,
penumbra_proto::DomainType,
penumbra_sct::component::clock::EpochRead,
penumbra_shielded_pool::SpendPlan,
penumbra_transaction::TransactionPlan,
penumbra_shielded_pool::{OutputPlan, SpendPlan},
penumbra_transaction::{
memo::MemoPlaintext,
plan::{CluePlan, DetectionDataPlan, MemoPlan},
ActionPlan, Transaction, TransactionParameters, TransactionPlan, WitnessData,
},
rand_core::OsRng,
std::ops::Deref,
tap::Tap,
tracing::{error_span, Instrument},
tracing::{error_span, info, Instrument},
};

/// Exercises that a test node can be instantiated using the consensus service.
Expand Down Expand Up @@ -80,54 +87,90 @@ async fn mock_consensus_can_send_a_spend_action() -> anyhow::Result<()> {
let guard = common::set_tracing_subscriber();
let storage = TempStorage::new().await?;
let mut test_node = common::start_test_node(&storage).await?;
let mut rng = <rand_chacha::ChaChaRng as rand_core::SeedableRng>::seed_from_u64(0xBEEF);

// Sync the mock client, using the test account's full viewing key, to the latest snapshot.
let (viewing_key, spend_key) = (&test_keys::FULL_VIEWING_KEY, &test_keys::SPEND_KEY);
let client = MockClient::new(viewing_key.deref().clone())
.with_sync_to_storage(&storage)
.await?;
.await?
.tap(|c| info!(client.notes = %c.notes.len(), "mock client synced to test storage"));

// Take one of the test account's notes...
let (commitment, note) = client
.notes
.iter()
.next()
.ok_or_else(|| anyhow!("mock client had no note"))?
.tap(|(commitment, note)| {
tracing::info!(?commitment, ?note, "mock client note commitment")
});
.tap(|(commitment, note)| info!(?commitment, ?note, "mock client note commitment"));

// Build a transaction spending this note.
let tx = {
let position = client
let spend: ActionPlan = {
let proof = client
.sct
.witness(*commitment)
.ok_or_else(|| anyhow!("commitment is not witnessed"))?
.position();
let spend = SpendPlan::new(&mut rng, note.clone(), position);
.witness(commitment.clone())
.ok_or_else(|| anyhow!("commitment is not witnessed"))?;
let note = note.clone();
let position = proof.position();
SpendPlan::new(&mut OsRng, note, position).into()
};

// Build a transaction spending this note.
let tx: Transaction = {
Copy link
Contributor Author

@cratelyn cratelyn Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a passing thought, but this part of the API is pretty long-winded. we talked a bit about this in our sprint planning meeting today, but i'd love to see a more convenient way to go about this for future test authors, without introducing a third planner API.

that is a design conversation that shouldn't block this PR, though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API we'd like to use is the Planner in the view crate. We'd need to be able to pass the MockClient to its plan() method: https://rustdoc.penumbra.zone/main/penumbra_view/struct.Planner.html#method.plan

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x-ref #3896, i think?

Copy link
Member

@hdevalence hdevalence Feb 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification post 54c0b35 , where the transaction plan creation becomes much simpler: my point is that for simple tests, where we can write down the complete transaction plan explicitly, we should do so. In this case, we can, which is what 54c0b35 does.

If we need to do more complex test logic, test logic that is complex enough that we cannot simply write down the plan, then we should decide on a way to use the Planner API to plan transactions, rather than have another set of helper methods for generating plans. This may involve pulling in the full rust view server implementation, or continuing to use the MockClient. But for simple cases it's important to be able to test the chain logic without all the complexity of the rust view server.

cratelyn marked this conversation as resolved.
Show resolved Hide resolved
let chain_id = TestNode::<()>::CHAIN_ID.to_string();
let transaction_parameters = TransactionParameters {
expiry_height: 0,
fee: Fee::default(),
chain_id,
};
let detection_data = Some(DetectionDataPlan {
clue_plans: vec![CluePlan::new(&mut OsRng, *test_keys::ADDRESS_0, 0)],
});
let plan = TransactionPlan {
actions: vec![spend.into()],
..Default::default()
actions: vec![
spend,
OutputPlan::new(&mut OsRng, note.value(), *test_keys::ADDRESS_1).into(),
],
transaction_parameters,
detection_data,
memo: Some(MemoPlan::new(
&mut OsRng,
MemoPlaintext::blank_memo(*test_keys::ADDRESS_1),
)?),
};
let witness = WitnessData {
anchor: client.sct.root(),
state_commitment_proofs: plan
.spend_plans()
.map(|spend| {
(
spend.note.commit(),
client.sct.witness(spend.note.commit()).unwrap(),
)
})
.collect(),
};
let witness = plan.witness_data(&client.sct)?;
let auth = plan.authorize(rand_core::OsRng, spend_key)?;
let auth = plan.authorize(OsRng, spend_key)?;
plan.build_concurrent(viewing_key, &witness, &auth).await?
};

// Execute the transaction, and sync another mock client up to the latest snapshot.
test_node
.block()
.with_data(vec![tx.encode_to_vec()]) // TODO(kate): add a `with_tx` extension method
.with_data(vec![tx.encode_to_vec()])
.execute()
.await?;

// Sync to the latest storage snapshot once more.
let client = MockClient::new(test_keys::FULL_VIEWING_KEY.clone())
let client_after_spend = MockClient::new(viewing_key.deref().clone())
.with_sync_to_storage(&storage)
.await?;
.await?
.tap(|c| info!(client.notes = %c.notes.len(), "mock client synced to test storage"));

client.notes.get(&commitment).unwrap();
// Show that we performed the spend as expected.
assert_eq!(
client_after_spend.notes.len(),
client.notes.len() + 1,
"a new note should exist after performing the spend",
);
cratelyn marked this conversation as resolved.
Show resolved Hide resolved

// Free our temporary storage.
drop(storage);
Expand Down
3 changes: 2 additions & 1 deletion crates/test/mock-consensus/src/builder/init_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,11 @@ impl Builder {

fn init_chain_request(app_state_bytes: Bytes) -> Result<ConsensusRequest, anyhow::Error> {
use tendermint::v0_37::abci::request::InitChain;
let chain_id = TestNode::<()>::CHAIN_ID.to_string();
let consensus_params = Self::consensus_params();
Ok(ConsensusRequest::InitChain(InitChain {
time: tendermint::Time::now(),
chain_id: "test".to_string(), // XXX const here?
chain_id,
consensus_params,
validators: vec![],
app_state_bytes,
Expand Down
2 changes: 2 additions & 0 deletions crates/test/mock-consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pub struct TestNode<C> {
}

impl<C> TestNode<C> {
pub const CHAIN_ID: &'static str = "penumbra-test-chain";

/// Returns the last app_hash value, as a slice of bytes.
pub fn last_app_hash(&self) -> &[u8] {
&self.last_app_hash
Expand Down
Loading