Skip to content

Commit

Permalink
tests(sequencer): add snapshot tests for app hash (#1057)
Browse files Browse the repository at this point in the history
## Summary
add snapshot tests for app hash to catch breaking changes made to the
app logic.

## Background
if changes are breaking, we should know about it. these tests should
catch (most) breaking changes to the application state itself.

## Changes
- add snapshot tests for `init_chain`, `finalize_block`, and all actions
except for IBC actions as they require extensive setup.

## Testing
no code was changed, just tests added

## Related Issues

closes #1051
  • Loading branch information
noot authored May 10, 2024
1 parent 171c765 commit 48a05a9
Show file tree
Hide file tree
Showing 7 changed files with 360 additions and 0 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/astria-sequencer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ astria-core = { path = "../astria-core", features = [
config = { package = "astria-config", path = "../astria-config", features = [
"tests",
] }
insta = { workspace = true, features = ["json"] }

[build-dependencies]
astria-build-info = { path = "../astria-build-info", features = ["build"] }
2 changes: 2 additions & 0 deletions crates/astria-sequencer/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ pub(crate) mod test_utils;
#[cfg(test)]
mod tests_app;
#[cfg(test)]
mod tests_breaking_changes;
#[cfg(test)]
mod tests_execute_transaction;

use std::{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
source: crates/astria-sequencer/src/app/tests_breaking_changes.rs
expression: app.app_hash.as_bytes()
---
[
191,
74,
54,
179,
5,
152,
196,
250,
125,
255,
173,
93,
58,
245,
0,
43,
100,
173,
142,
245,
56,
191,
70,
2,
181,
214,
161,
226,
13,
171,
20,
255
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
source: crates/astria-sequencer/src/app/tests_breaking_changes.rs
expression: app.app_hash.as_bytes()
---
[
110,
57,
239,
200,
69,
178,
164,
1,
209,
124,
151,
238,
176,
163,
203,
213,
34,
90,
125,
33,
11,
226,
74,
29,
92,
21,
38,
30,
218,
176,
165,
209
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
source: crates/astria-sequencer/src/app/tests_breaking_changes.rs
expression: app.app_hash.as_bytes()
---
[
219,
22,
109,
121,
104,
40,
134,
239,
14,
66,
234,
223,
96,
123,
174,
58,
255,
111,
202,
30,
237,
96,
84,
181,
106,
173,
7,
3,
13,
206,
123,
228
]
242 changes: 242 additions & 0 deletions crates/astria-sequencer/src/app/tests_breaking_changes.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
//! The tests in this file are snapshot tests that are expected to break
//! if breaking changes are made to the application, in particular,
//! any changes that can affect the state tree and thus the app hash.
//!
//! If these tests break due to snapshot mismatches, you can update the snapshots
//! with `cargo insta review`, but you MUST mark the respective PR as breaking.
//!
//! Note: there are two actions not tested here: `Ics20Withdrawal` and `IbcRelay`.
//! These are due to the extensive setup needed to test them.
//! If changes are made to the execution results of these actions, manual testing is required.
use std::collections::HashMap;

use astria_core::{
primitive::v1::{
asset::DEFAULT_NATIVE_ASSET_DENOM,
Address,
RollupId,
},
protocol::transaction::v1alpha1::{
action::{
BridgeLockAction,
IbcRelayerChangeAction,
SequenceAction,
TransferAction,
},
Action,
TransactionParams,
UnsignedTransaction,
},
sequencerblock::v1alpha1::block::Deposit,
};
use cnidarium::StateDelta;
use penumbra_ibc::params::IBCParameters;
use prost::Message as _;
use tendermint::{
abci,
abci::types::CommitInfo,
block::Round,
Hash,
Time,
};

use crate::{
app::test_utils::{
address_from_hex_string,
default_fees,
default_genesis_accounts,
get_alice_signing_key_and_address,
initialize_app,
initialize_app_with_storage,
BOB_ADDRESS,
CAROL_ADDRESS,
},
asset::get_native_asset,
bridge::state_ext::StateWriteExt as _,
genesis::GenesisState,
proposal::commitment::generate_rollup_datas_commitment,
};

#[tokio::test]
async fn app_genesis_snapshot() {
let app = initialize_app(None, vec![]).await;
insta::assert_json_snapshot!(app.app_hash.as_bytes());
}

#[tokio::test]
async fn app_finalize_block_snapshot() {
let (alice_signing_key, _) = get_alice_signing_key_and_address();
let (mut app, storage) = initialize_app_with_storage(None, vec![]).await;

let bridge_address = Address::from([99; 20]);
let rollup_id = RollupId::from_unhashed_bytes(b"testchainid");
let asset_id = get_native_asset().id();

let mut state_tx = StateDelta::new(app.state.clone());
state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id);
state_tx
.put_bridge_account_asset_id(&bridge_address, &asset_id)
.unwrap();
app.apply(state_tx);

// the state changes must be committed, as `finalize_block` will execute the
// changes on the latest snapshot, not the app's `StateDelta`.
app.prepare_commit(storage.clone()).await.unwrap();
app.commit(storage.clone()).await;

let amount = 100;
let lock_action = BridgeLockAction {
to: bridge_address,
amount,
asset_id,
fee_asset_id: asset_id,
destination_chain_address: "nootwashere".to_string(),
};
let sequence_action = SequenceAction {
rollup_id,
data: b"hello world".to_vec(),
fee_asset_id: asset_id,
};
let tx = UnsignedTransaction {
params: TransactionParams {
nonce: 0,
chain_id: "test".to_string(),
},
actions: vec![lock_action.into(), sequence_action.into()],
};

let signed_tx = tx.into_signed(&alice_signing_key);

let expected_deposit = Deposit::new(
bridge_address,
rollup_id,
amount,
asset_id,
"nootwashere".to_string(),
);
let deposits = HashMap::from_iter(vec![(rollup_id, vec![expected_deposit.clone()])]);
let commitments = generate_rollup_datas_commitment(&[signed_tx.clone()], deposits.clone());

let timestamp = Time::now();
let block_hash = Hash::try_from([99u8; 32].to_vec()).unwrap();
let finalize_block = abci::request::FinalizeBlock {
hash: block_hash,
height: 1u32.into(),
time: timestamp,
next_validators_hash: Hash::default(),
proposer_address: [0u8; 20].to_vec().try_into().unwrap(),
txs: commitments.into_transactions(vec![signed_tx.to_raw().encode_to_vec().into()]),
decided_last_commit: CommitInfo {
votes: vec![],
round: Round::default(),
},
misbehavior: vec![],
};

app.finalize_block(finalize_block.clone(), storage.clone())
.await
.unwrap();
insta::assert_json_snapshot!(app.app_hash.as_bytes());
}

// Note: this tests every action except for `Ics20Withdrawal` and `IbcRelay`.
//
// If new actions are added to the app, they must be added to this test,
// and the respective PR must be marked as breaking.
#[tokio::test]
async fn app_execute_transaction_with_every_action_snapshot() {
use astria_core::{
primitive::v1::asset,
protocol::transaction::v1alpha1::action::{
FeeAssetChangeAction,
InitBridgeAccountAction,
SudoAddressChangeAction,
},
};

let (alice_signing_key, alice_address) = get_alice_signing_key_and_address();
let bob_address = address_from_hex_string(BOB_ADDRESS);
let carol_address = address_from_hex_string(CAROL_ADDRESS);

let genesis_state = GenesisState {
accounts: default_genesis_accounts(),
authority_sudo_address: alice_address,
ibc_sudo_address: alice_address,
ibc_relayer_addresses: vec![],
native_asset_base_denomination: DEFAULT_NATIVE_ASSET_DENOM.to_string(),
ibc_params: IBCParameters::default(),
allowed_fee_assets: vec![DEFAULT_NATIVE_ASSET_DENOM.to_owned().into()],
fees: default_fees(),
};
let mut app = initialize_app(Some(genesis_state), vec![]).await;

// setup for ValidatorUpdate action
let pub_key = tendermint::public_key::PublicKey::from_raw_ed25519(&[1u8; 32]).unwrap();
let update = tendermint::validator::Update {
pub_key,
power: 100u32.into(),
};

// setup for BridgeLockAction
let bridge_address = Address::from([99; 20]);
let rollup_id = RollupId::from_unhashed_bytes(b"testchainid");
let asset_id = get_native_asset().id();
let mut state_tx = StateDelta::new(app.state.clone());
state_tx.put_bridge_account_rollup_id(&bridge_address, &rollup_id);
state_tx
.put_bridge_account_asset_id(&bridge_address, &asset_id)
.unwrap();
app.apply(state_tx);

let tx = UnsignedTransaction {
params: TransactionParams {
nonce: 0,
chain_id: "test".to_string(),
},
actions: vec![
TransferAction {
to: bob_address,
amount: 333_333,
asset_id: get_native_asset().id(),
fee_asset_id: get_native_asset().id(),
}
.into(),
SequenceAction {
rollup_id: RollupId::from_unhashed_bytes(b"testchainid"),
data: b"hello world".to_vec(),
fee_asset_id: get_native_asset().id(),
}
.into(),
Action::ValidatorUpdate(update.clone()),
IbcRelayerChangeAction::Addition(bob_address).into(),
IbcRelayerChangeAction::Addition(carol_address).into(),
IbcRelayerChangeAction::Removal(bob_address).into(),
// TODO: should fee assets be stored in state?
FeeAssetChangeAction::Addition(asset::Id::from("test-0".to_string())).into(),
FeeAssetChangeAction::Addition(asset::Id::from("test-1".to_string())).into(),
FeeAssetChangeAction::Removal(asset::Id::from("test-0".to_string())).into(),
InitBridgeAccountAction {
rollup_id: RollupId::from_unhashed_bytes(b"testchainid"),
asset_id: get_native_asset().id(),
fee_asset_id: get_native_asset().id(),
}
.into(),
BridgeLockAction {
to: bridge_address,
amount: 100,
asset_id: get_native_asset().id(),
fee_asset_id: get_native_asset().id(),
destination_chain_address: "nootwashere".to_string(),
}
.into(),
SudoAddressChangeAction {
new_address: bob_address,
}
.into(),
],
};

let signed_tx = tx.into_signed(&alice_signing_key);
app.execute_transaction(signed_tx).await.unwrap();
insta::assert_json_snapshot!(app.app_hash.as_bytes());
}

0 comments on commit 48a05a9

Please sign in to comment.