Skip to content

Commit

Permalink
tx: test and refactor tx sections api
Browse files Browse the repository at this point in the history
  • Loading branch information
tzemanovic committed Sep 23, 2024
1 parent 2a76518 commit 98be5f3
Show file tree
Hide file tree
Showing 27 changed files with 270 additions and 153 deletions.
4 changes: 2 additions & 2 deletions crates/benches/native_vps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1684,7 +1684,7 @@ fn ibc_vp_validate_action(c: &mut Criterion) {

let verifiers_from_tx =
shielded_ctx.shell.write().execute_tx(&signed_tx.to_ref());
let tx_data = signed_tx.tx.data(&signed_tx.cmt).unwrap();
let tx_data = signed_tx.to_ref().data().unwrap();
let shell_read = shielded_ctx.shell.read();
let (verifiers, keys_changed) = shell_read
.state
Expand Down Expand Up @@ -1745,7 +1745,7 @@ fn ibc_vp_execute_action(c: &mut Criterion) {
let verifiers_from_tx =
shielded_ctx.shell.write().execute_tx(&signed_tx.to_ref());
let shell_read = shielded_ctx.shell.read();
let tx_data = signed_tx.tx.data(&signed_tx.cmt).unwrap();
let tx_data = signed_tx.to_ref().data().unwrap();
let (verifiers, keys_changed) = shell_read
.state
.write_log()
Expand Down
2 changes: 1 addition & 1 deletion crates/ethereum_bridge/src/vp/bridge_pool_vp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ where
"Rejecting transaction, since the Ethereum bridge is disabled.",
));
}
let Some(tx_data) = batched_tx.tx.data(batched_tx.cmt) else {
let Some(tx_data) = batched_tx.data() else {
return Err(Error::new_const("No transaction data found"));
};
let transfer: PendingTransfer =
Expand Down
11 changes: 4 additions & 7 deletions crates/governance/src/vp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ where
/// Run the validity predicate
pub fn validate_tx(
ctx: &'ctx CTX,
tx_data: &BatchedTxRef<'_>,
tx: &BatchedTxRef<'_>,
keys_changed: &BTreeSet<storage::Key>,
verifiers: &BTreeSet<Address>,
) -> Result<()> {
Expand All @@ -74,7 +74,7 @@ where
// Is VP triggered by a governance proposal?
if is_proposal_accepted(
&ctx.pre(),
tx_data.tx.data(tx_data.cmt).unwrap_or_default().as_ref(),
tx.data().unwrap_or_default().as_ref(),
)? {
return Ok(());
}
Expand Down Expand Up @@ -168,9 +168,7 @@ where
(KeyType::PROPOSAL_COMMIT, _) => {
Self::is_valid_proposal_commit(ctx)
}
(KeyType::PARAMETER, _) => {
Self::is_valid_parameter(ctx, tx_data)
}
(KeyType::PARAMETER, _) => Self::is_valid_parameter(ctx, tx),
(KeyType::BALANCE, _) => {
Self::is_valid_balance(ctx, &native_token)
}
Expand Down Expand Up @@ -988,8 +986,7 @@ where
ctx: &'ctx CTX,
batched_tx: &BatchedTxRef<'_>,
) -> Result<()> {
let BatchedTxRef { tx, cmt } = batched_tx;
tx.data(cmt).map_or_else(
batched_tx.data().map_or_else(
|| {
Err(Error::new_const(
"Governance parameter changes require tx data to be \
Expand Down
8 changes: 2 additions & 6 deletions crates/governance/src/vp/pgf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,7 @@ where
// Is VP triggered by a governance proposal?
if is_proposal_accepted(
&ctx.pre(),
batched_tx
.tx
.data(batched_tx.cmt)
.unwrap_or_default()
.as_ref(),
batched_tx.data().unwrap_or_default().as_ref(),
)? {
return Ok(());
}
Expand Down Expand Up @@ -189,7 +185,7 @@ where
ctx: &'ctx CTX,
batched_tx: &BatchedTxRef<'_>,
) -> Result<()> {
batched_tx.tx.data(batched_tx.cmt).map_or_else(
batched_tx.data().map_or_else(
|| {
Err(Error::new_const(
"PGF parameter changes require tx data to be present",
Expand Down
11 changes: 2 additions & 9 deletions crates/ibc/src/vp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,19 +148,12 @@ where
// Is VP triggered by a governance proposal?
if Gov::is_proposal_accepted(
&self.ctx.pre(),
batched_tx
.tx
.data(batched_tx.cmt)
.unwrap_or_default()
.as_ref(),
batched_tx.data().unwrap_or_default().as_ref(),
)? {
return Ok(());
}

let tx_data = batched_tx
.tx
.data(batched_tx.cmt)
.ok_or(VpError::NoTxData)?;
let tx_data = batched_tx.data().ok_or(VpError::NoTxData)?;

// Pseudo execution and compare them
self.validate_state(&tx_data, keys_changed)?;
Expand Down
2 changes: 1 addition & 1 deletion crates/node/src/bench_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1338,7 +1338,7 @@ impl BenchShieldedCtx {
};

let vectorized_transfer =
Transfer::deserialize(&mut tx.tx.data(&tx.cmt).unwrap().as_slice())
Transfer::deserialize(&mut tx.to_ref().data().unwrap().as_slice())
.unwrap();
let sources =
vec![vectorized_transfer.sources.into_iter().next().unwrap()]
Expand Down
9 changes: 6 additions & 3 deletions crates/node/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ use namada_sdk::tx::data::{
BatchedTxResult, ExtendedTxResult, TxResult, VpStatusFlags, VpsResult,
WrapperTx,
};
use namada_sdk::tx::{BatchedTxRef, Tx, TxCommitments};
use namada_sdk::tx::{BatchedTxRef, InnerTxRef, Tx, TxCommitments};
use namada_sdk::validation::{
EthBridgeNutVp, EthBridgePoolVp, EthBridgeVp, GovernanceVp, IbcVp, MaspVp,
MultitokenVp, NativeVpCtx, ParametersVp, PgfVp, PosVp,
Expand Down Expand Up @@ -272,8 +272,11 @@ where
DispatchArgs::Protocol(protocol_tx) => {
// No bundles of protocol transactions, only take the first one
let cmt = tx.first_commitments().ok_or(Error::MissingInnerTxs)?;
let batched_tx_result =
apply_protocol_tx(protocol_tx.tx, tx.data(cmt), state)?;
let batched_tx_result = apply_protocol_tx(
protocol_tx.tx,
tx.data(InnerTxRef::Commitment(cmt)),
state,
)?;

Ok({
let mut batch_results = TxResult::new();
Expand Down
2 changes: 1 addition & 1 deletion crates/parameters/src/vp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ where
) -> Result<()> {
keys_changed.iter().try_for_each(|key| {
let key_type: KeyType = key.into();
let data = if let Some(data) = batched_tx.tx.data(batched_tx.cmt) {
let data = if let Some(data) = batched_tx.data() {
data
} else {
return Err(Error::new_const(
Expand Down
4 changes: 2 additions & 2 deletions crates/proof_of_stake/src/vp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use namada_systems::governance;
use namada_tx::action::{
Action, Bond, ClaimRewards, PosAction, Redelegation, Unbond, Withdraw,
};
use namada_tx::BatchedTxRef;
use namada_tx::{BatchedTxRef, InnerTxRef};
use namada_vp_env::{Error, Result, VpEnv};
use thiserror::Error;

Expand Down Expand Up @@ -57,7 +57,7 @@ where
// Check if this is a governance proposal first
if batched_tx
.tx
.data(batched_tx.cmt)
.data(InnerTxRef::Index(0))
.map(|tx_data| Gov::is_proposal_accepted(&ctx.pre(), &tx_data))
.transpose()?
.unwrap_or(false)
Expand Down
15 changes: 4 additions & 11 deletions crates/sdk/src/masp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use namada_ibc::{decode_message, extract_masp_tx_from_envelope, IbcMessage};
use namada_io::client::Client;
use namada_token::masp::shielded_wallet::ShieldedQueries;
pub use namada_token::masp::{utils, *};
use namada_tx::Tx;
use namada_tx::{InnerTxRef, Tx};
pub use utilities::{IndexerMaspClient, LedgerMaspClient};

use crate::error::{Error, QueryError};
Expand Down Expand Up @@ -72,20 +72,13 @@ fn extract_masp_tx(
// Dereference the masp ref to the first instance that
// matches is, even if it is not the exact one that produced
// the event, the data we extract will be exactly the same
let masp_ibc_tx = tx
.commitments()
.iter()
.find(|cmt| cmt.data_sechash() == hash)
let tx_data = tx
.data(InnerTxRef::CommitmentHash(*hash))
.ok_or_else(|| {
Error::Other(format!(
"Couldn't find data section with hash {hash}"
"Couldn't find data section with hash {hash}",
))
})?;
let tx_data = tx.data(masp_ibc_tx).ok_or_else(|| {
Error::Other(
"Missing expected data section".to_string(),
)
})?;

let IbcMessage::Envelope(envelope) =
decode_message::<token::Transfer>(&tx_data)
Expand Down
Loading

0 comments on commit 98be5f3

Please sign in to comment.