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

Fix IBC MASP tx extraction #3490

Merged
merged 3 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Fix to extract MASP transaction when IBC shielding transfer
([\#3488](https://github.com/anoma/namada/issues/3488))
23 changes: 23 additions & 0 deletions crates/core/src/ibc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! IBC-related data types

use std::fmt::Display;
use std::str::FromStr;

use borsh::{BorshDeserialize, BorshSchema, BorshSerialize};
Expand All @@ -11,6 +12,7 @@ use namada_migrations::*;
use serde::{Deserialize, Serialize};

use super::address::HASH_LEN;
use crate::hash::Hash;

/// IBC token hash derived from a denomination.
#[derive(
Expand Down Expand Up @@ -47,3 +49,24 @@ impl FromStr for IbcTokenHash {
Ok(IbcTokenHash(output))
}
}

/// IBC transaction data section hash
pub type IbcTxDataHash = Hash;

/// IBC transaction data references to retrieve IBC messages
#[derive(Default, Clone, Serialize, Deserialize)]
pub struct IbcTxDataRefs(pub Vec<IbcTxDataHash>);

impl Display for IbcTxDataRefs {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{}", serde_json::to_string(self).unwrap())
}
}

impl FromStr for IbcTxDataRefs {
type Err = serde_json::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
serde_json::from_str(s)
}
}
15 changes: 15 additions & 0 deletions crates/events/src/extend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::str::FromStr;
use namada_core::address::Address;
use namada_core::collections::HashMap;
use namada_core::hash::Hash;
use namada_core::ibc::IbcTxDataRefs;
use namada_core::masp::MaspTxRefs;
use namada_core::storage::{BlockHeight, TxIndex};
use serde::Deserializer;
Expand Down Expand Up @@ -517,6 +518,20 @@ impl EventAttributeEntry<'static> for MaspTxBatchRefs {
}
}

/// Extend an [`Event`] with data sections for IBC shielding transfer.
pub struct IbcMaspTxBatchRefs(pub IbcTxDataRefs);

impl EventAttributeEntry<'static> for IbcMaspTxBatchRefs {
type Value = IbcTxDataRefs;
type ValueOwned = Self::Value;

const KEY: &'static str = "ibc_masp_tx_batch_refs";

fn into_value(self) -> Self::Value {
self.0
}
}

/// Extend an [`Event`] with success data.
pub struct Success(pub bool);

Expand Down
12 changes: 7 additions & 5 deletions crates/namada/src/ledger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,17 @@ mod dry_run_tx {
let ExtendedTxResult {
mut tx_result,
ref masp_tx_refs,
is_ibc_shielding: _,
ref ibc_tx_data_refs,
} = extended_tx_result;
let tx_gas_meter = RefCell::new(tx_gas_meter);
for cmt in
crate::ledger::protocol::get_batch_txs_to_execute(&tx, masp_tx_refs)
{
for cmt in crate::ledger::protocol::get_batch_txs_to_execute(
&tx,
masp_tx_refs,
ibc_tx_data_refs,
) {
let batched_tx = tx.batch_ref_tx(cmt);
let batched_tx_result = protocol::apply_wasm_tx(
batched_tx,
&batched_tx,
&TxIndex(0),
ShellParams::new(
&tx_gas_meter,
Expand Down
99 changes: 70 additions & 29 deletions crates/namada/src/ledger/protocol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ use std::cell::RefCell;
use std::collections::BTreeSet;
use std::fmt::Debug;

use either::Either;
use eyre::{eyre, WrapErr};
use namada_core::booleans::BoolResultUnitExt;
use namada_core::hash::Hash;
use namada_core::ibc::{IbcTxDataHash, IbcTxDataRefs};
use namada_core::masp::{MaspTxRefs, TxId};
use namada_events::extend::{
ComposeEvent, Height as HeightAttr, TxHash as TxHashAttr, UserAccount,
Expand All @@ -16,7 +18,7 @@ use namada_parameters::get_gas_scale;
use namada_state::TxWrites;
use namada_token::event::{TokenEvent, TokenOperation};
use namada_token::utils::is_masp_transfer;
use namada_tx::action::Read;
use namada_tx::action::{is_ibc_shielding_transfer, Read};
use namada_tx::data::protocol::{ProtocolTx, ProtocolTxType};
use namada_tx::data::{
BatchResults, BatchedTxResult, ExtendedTxResult, TxResult, VpStatusFlags,
Expand Down Expand Up @@ -265,7 +267,7 @@ where
let cmt =
tx.first_commitments().ok_or(Error::MissingInnerTxs)?;
let batched_tx_result = apply_wasm_tx(
tx.batch_ref_tx(cmt),
&tx.batch_ref_tx(cmt),
&tx_index,
ShellParams {
tx_gas_meter,
Expand Down Expand Up @@ -342,9 +344,10 @@ where
pub(crate) fn get_batch_txs_to_execute<'a>(
tx: &'a Tx,
masp_tx_refs: &MaspTxRefs,
ibc_tx_data_refs: &IbcTxDataRefs,
) -> impl Iterator<Item = &'a TxCommitments> {
let mut batch_iter = tx.commitments().iter();
if !masp_tx_refs.0.is_empty() {
if !masp_tx_refs.0.is_empty() || !ibc_tx_data_refs.0.is_empty() {
// If fees were paid via masp skip the first transaction of the batch
// which has already been executed
batch_iter.next();
Expand All @@ -369,9 +372,13 @@ where
H: 'static + StorageHasher + Sync,
CA: 'static + WasmCacheAccess + Sync,
{
for cmt in get_batch_txs_to_execute(tx, &extended_tx_result.masp_tx_refs) {
for cmt in get_batch_txs_to_execute(
tx,
&extended_tx_result.masp_tx_refs,
&extended_tx_result.ibc_tx_data_refs,
) {
match apply_wasm_tx(
tx.batch_ref_tx(cmt),
&tx.batch_ref_tx(cmt),
&tx_index,
ShellParams {
tx_gas_meter,
Expand Down Expand Up @@ -424,9 +431,14 @@ where
.0
.push(masp_section_ref);
}
extended_tx_result.is_ibc_shielding =
namada_tx::action::is_ibc_shielding_transfer(state)
.map_err(Error::StateError)?;
if is_ibc_shielding_transfer(&*state)
.map_err(Error::StateError)?
{
extended_tx_result
.ibc_tx_data_refs
.0
.push(*cmt.data_sechash())
}
state.write_log_mut().commit_tx_to_batch();
} else {
state.write_log_mut().drop_tx();
Expand All @@ -447,6 +459,12 @@ where
Ok(extended_tx_result)
}

/// Transaction result for masp transfer
pub struct MaspTxResult {
tx_result: BatchedTxResult,
masp_section_ref: Either<TxId, IbcTxDataHash>,
}

/// Performs the required operation on a wrapper transaction:
/// - replay protection
/// - fee payment
Expand Down Expand Up @@ -487,18 +505,23 @@ where
// have propagated an error)
shell_params.state.write_log_mut().commit_batch();

let (batch_results, masp_tx_refs) = payment_result.map_or_else(
let (batch_results, masp_section_refs) = payment_result.map_or_else(
|| (BatchResults::default(), None),
|(batched_result, masp_section_ref)| {
|masp_tx_result| {
let mut batch = BatchResults::default();
batch.insert_inner_tx_result(
// Ok to unwrap cause if we have a batched result it means
// we've executed the first tx in the batch
tx.wrapper_hash().as_ref(),
either::Right(tx.first_commitments().unwrap()),
Ok(batched_result),
Ok(masp_tx_result.tx_result),
);
(batch, Some(MaspTxRefs(vec![masp_section_ref])))
let masp_section_refs =
Some(masp_tx_result.masp_section_ref.map_either(
|masp_tx_ref| MaspTxRefs(vec![masp_tx_ref]),
|ibc_tx_data| IbcTxDataRefs(vec![ibc_tx_data]),
));
(batch, masp_section_refs)
},
);

Expand All @@ -512,7 +535,7 @@ where
gas_used: tx_gas_meter.borrow().get_tx_consumed_gas(),
batch_results,
}
.to_extended_result(masp_tx_refs))
.to_extended_result(masp_section_refs))
}

/// Perform the actual transfer of fees from the fee payer to the block
Expand All @@ -524,7 +547,7 @@ pub fn transfer_fee<S, D, H, CA>(
tx: &Tx,
wrapper: &WrapperTx,
tx_index: &TxIndex,
) -> Result<Option<(BatchedTxResult, TxId)>>
) -> Result<Option<MaspTxResult>>
where
S: State<D = D, H = H>
+ StorageRead
Expand Down Expand Up @@ -677,7 +700,7 @@ fn try_masp_fee_payment<S, D, H, CA>(
}: &mut ShellParams<'_, S, D, H, CA>,
tx: &Tx,
tx_index: &TxIndex,
) -> Result<Option<(BatchedTxResult, TxId)>>
) -> Result<Option<MaspTxResult>>
where
S: State<D = D, H = H>
+ StorageRead
Expand Down Expand Up @@ -713,9 +736,11 @@ where
let ref_unshield_gas_meter = RefCell::new(gas_meter);

let valid_batched_tx_result = {
let first_tx = tx
.batch_ref_first_tx()
.ok_or_else(|| Error::MissingInnerTxs)?;
match apply_wasm_tx(
tx.batch_ref_first_tx()
.ok_or_else(|| Error::MissingInnerTxs)?,
&first_tx,
tx_index,
ShellParams {
tx_gas_meter: &ref_unshield_gas_meter,
Expand All @@ -740,16 +765,32 @@ where
);
}

let masp_ref = namada_tx::action::get_masp_section_ref(*state)
.map_err(Error::StateError)?;
// Ensure that the transaction is actually a masp one, otherwise
// reject
(is_masp_transfer(&result.changed_keys) && result.is_accepted())
.then(|| {
masp_ref
.map(|masp_section_ref| (result, masp_section_ref))
})
.flatten()
if is_masp_transfer(&result.changed_keys)
&& result.is_accepted()
{
if let Some(masp_tx_id) =
namada_tx::action::get_masp_section_ref(*state)
.map_err(Error::StateError)?
{
Some(MaspTxResult {
tx_result: result,
masp_section_ref: Either::Left(masp_tx_id),
})
} else {
is_ibc_shielding_transfer(*state)
.map_err(Error::StateError)?
.then_some(MaspTxResult {
tx_result: result,
masp_section_ref: Either::Right(
*first_tx.cmt.data_sechash(),
),
})
}
} else {
None
}
}
Err(e) => {
state.write_log_mut().drop_tx();
Expand Down Expand Up @@ -807,7 +848,7 @@ pub fn check_fees<S, D, H, CA>(
shell_params: &mut ShellParams<'_, S, D, H, CA>,
tx: &Tx,
wrapper: &WrapperTx,
) -> Result<Option<(BatchedTxResult, TxId)>>
) -> Result<Option<MaspTxResult>>
where
S: State<D = D, H = H>
+ StorageRead
Expand Down Expand Up @@ -877,7 +918,7 @@ where
/// Apply a transaction going via the wasm environment. Gas will be metered and
/// validity predicates will be triggered in the normal way.
pub fn apply_wasm_tx<'a, S, D, H, CA>(
batched_tx: BatchedTxRef<'_>,
batched_tx: &BatchedTxRef<'_>,
tx_index: &TxIndex,
shell_params: ShellParams<'a, S, D, H, CA>,
) -> Result<BatchedTxResult>
Expand All @@ -895,7 +936,7 @@ where
} = shell_params;

let verifiers = execute_tx(
&batched_tx,
batched_tx,
tx_index,
state,
tx_gas_meter,
Expand All @@ -904,7 +945,7 @@ where
)?;

let vps_result = check_vps(CheckVps {
batched_tx: &batched_tx,
batched_tx,
tx_index,
state,
tx_gas_meter: &mut tx_gas_meter.borrow_mut(),
Expand Down
8 changes: 6 additions & 2 deletions crates/node/src/shell/finalize_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ use namada::gas::event::GasUsed;
use namada::governance::pgf::inflation as pgf_inflation;
use namada::hash::Hash;
use namada::ledger::events::extend::{
ComposeEvent, Height, Info, MaspTxBatchRefs, MaspTxBlockIndex, TxHash,
ComposeEvent, Height, IbcMaspTxBatchRefs, Info, MaspTxBatchRefs,
MaspTxBlockIndex, TxHash,
};
use namada::ledger::events::EmitEvents;
use namada::ledger::gas::GasMetering;
Expand Down Expand Up @@ -1056,9 +1057,12 @@ impl<'finalize> TempTxLogs {
));
}

if extended_tx_result.is_ibc_shielding {
if !extended_tx_result.ibc_tx_data_refs.0.is_empty() {
self.tx_event
.extend(MaspTxBlockIndex(TxIndex::must_from_usize(tx_index)));
self.tx_event.extend(IbcMaspTxBatchRefs(
extended_tx_result.ibc_tx_data_refs.clone(),
));
}

flags
Expand Down
Loading
Loading