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

tx_ibc wasm again #3275

Merged
merged 5 commits into from
May 24, 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
3 changes: 3 additions & 0 deletions .changelog/unreleased/improvements/1831-tx-ibc-wasm.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Revert IBC transaction wasm not to use host_env function.
But it required to enable floating-point support again
([\#1831](https://github.com/anoma/namada/issues/1831))
97 changes: 9 additions & 88 deletions crates/ibc/src/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ use namada_events::{EmitEvents, EventTypeBuilder};
use namada_governance::storage::proposal::PGFIbcTarget;
use namada_parameters::read_epoch_duration_parameter;
use namada_state::{
DBIter, Epochs, OptionExt, ResultExt, State, StateRead, StorageError,
StorageHasher, StorageRead, StorageResult, StorageWrite, TxHostEnvState,
WlState, DB,
DBIter, Epochs, ResultExt, State, StorageError, StorageHasher, StorageRead,
StorageResult, StorageWrite, WlState, DB,
};
use namada_token as token;
use token::DenominatedAmount;
Expand Down Expand Up @@ -115,83 +114,6 @@ where
}
}

impl<D, H> IbcStorageContext for TxHostEnvState<'_, D, H>
where
D: 'static + DB + for<'iter> DBIter<'iter>,
H: 'static + StorageHasher,
{
fn emit_ibc_event(&mut self, event: IbcEvent) -> Result<(), StorageError> {
let gas = self
.write_log_mut()
.emit_event(event)
.ok_or_err_msg("Gas overflow")?;
self.charge_gas(gas).into_storage_result()?;
Ok(())
}

fn get_ibc_events(
&self,
event_type: impl AsRef<str>,
) -> Result<Vec<IbcEvent>, StorageError> {
let event_type = EventTypeBuilder::new_of::<IbcEvent>()
.with_segment(event_type)
.build();

Ok(self
.write_log()
.lookup_events_with_prefix(&event_type)
.filter_map(|event| IbcEvent::try_from(event).ok())
.collect())
}

fn transfer_token(
&mut self,
src: &Address,
dest: &Address,
token: &Address,
amount: Amount,
) -> Result<(), StorageError> {
token::transfer(self, token, src, dest, amount)
}

fn handle_masp_tx(
&mut self,
shielded: &masp_primitives::transaction::Transaction,
) -> Result<(), StorageError> {
namada_token::utils::handle_masp_tx(self, shielded)?;
namada_token::utils::update_note_commitment_tree(self, shielded)
}

fn mint_token(
&mut self,
target: &Address,
token: &Address,
amount: Amount,
) -> Result<(), StorageError> {
ibc_storage::mint_tokens(self, target, token, amount)
}

fn burn_token(
&mut self,
target: &Address,
token: &Address,
amount: Amount,
) -> Result<(), StorageError> {
ibc_storage::burn_tokens(self, target, token, amount)
}

fn log_string(&self, message: String) {
tracing::trace!(message);
}
}

impl<D, H> IbcCommonContext for TxHostEnvState<'_, D, H>
where
D: 'static + DB + for<'iter> DBIter<'iter>,
H: 'static + StorageHasher,
{
}

impl<S> IbcStorageContext for IbcProtocolContext<'_, S>
where
S: State + EmitEvents,
Expand Down Expand Up @@ -230,14 +152,6 @@ where
token::transfer(self.state, token, src, dest, amount)
}

/// Handle masp tx
fn handle_masp_tx(
&mut self,
_shielded: &masp_primitives::transaction::Transaction,
) -> Result<(), StorageError> {
unimplemented!("No MASP transfer in an IBC protocol transaction")
}

/// Mint token
fn mint_token(
&mut self,
Expand All @@ -258,6 +172,13 @@ where
ibc_storage::burn_tokens(self.state, target, token, amount)
}

fn insert_verifier(
&mut self,
_verifier: &Address,
) -> Result<(), StorageError> {
Ok(())
}

fn log_string(&self, message: String) {
tracing::trace!(message);
}
Expand Down
9 changes: 3 additions & 6 deletions crates/ibc/src/context/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,6 @@ pub trait IbcStorageContext: StorageRead + StorageWrite {
amount: Amount,
) -> Result<(), Error>;

/// Handle masp tx
fn handle_masp_tx(
&mut self,
shielded: &masp_primitives::transaction::Transaction,
) -> Result<(), Error>;

/// Mint token
fn mint_token(
&mut self,
Expand All @@ -49,6 +43,9 @@ pub trait IbcStorageContext: StorageRead + StorageWrite {
amount: Amount,
) -> Result<(), Error>;

/// Insert the verifier
fn insert_verifier(&mut self, verifier: &Address) -> Result<(), Error>;

/// Logging
fn log_string(&self, message: String);
}
14 changes: 12 additions & 2 deletions crates/ibc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ pub enum Error {
Trace(String),
#[error("Invalid chain ID: {0}")]
ChainId(IdentifierError),
#[error("Handling MASP transaction error: {0}")]
MaspTx(String),
#[error("Verifier insertion error: {0}")]
Verifier(namada_storage::Error),
}

/// IBC actions to handle IBC operations
Expand Down Expand Up @@ -150,6 +150,7 @@ where
self.ctx.inner.clone(),
self.verifiers.clone(),
);
self.insert_verifiers()?;
send_transfer_execute(
&mut self.ctx,
&mut token_transfer_ctx,
Expand Down Expand Up @@ -366,6 +367,7 @@ where
self.ctx.inner.clone(),
verifiers.clone(),
);
self.insert_verifiers()?;
send_transfer_validate(
&self.ctx,
&token_transfer_ctx,
Expand Down Expand Up @@ -407,6 +409,14 @@ where
}
}
}

fn insert_verifiers(&self) -> Result<(), Error> {
let mut ctx = self.ctx.inner.borrow_mut();
for verifier in self.verifiers.borrow().iter() {
ctx.insert_verifier(verifier).map_err(Error::Verifier)?;
}
Ok(())
}
}

fn is_ack_successful(ack: &Acknowledgement) -> Result<bool, Error> {
Expand Down
23 changes: 8 additions & 15 deletions crates/namada/src/ledger/native_vp/ibc/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,6 @@ where
transfer(self, token, src, dest, amount)
}

fn handle_masp_tx(
&mut self,
shielded: &masp_primitives::transaction::Transaction,
) -> Result<()> {
crate::token::utils::handle_masp_tx(self, shielded)?;
crate::token::utils::update_note_commitment_tree(self, shielded)
}

fn mint_token(
&mut self,
target: &Address,
Expand All @@ -259,6 +251,10 @@ where
burn_tokens(self, token, target, amount)
}

fn insert_verifier(&mut self, _verifier: &Address) -> Result<()> {
Ok(())
}

fn log_string(&self, message: String) {
tracing::debug!("{message} in the pseudo execution for IBC VP");
}
Expand Down Expand Up @@ -397,13 +393,6 @@ where
unimplemented!("Validation doesn't transfer")
}

fn handle_masp_tx(
&mut self,
_shielded: &masp_primitives::transaction::Transaction,
) -> Result<()> {
unimplemented!("Validation doesn't handle a masp tx")
}

fn mint_token(
&mut self,
_target: &Address,
Expand All @@ -422,6 +411,10 @@ where
unimplemented!("Validation doesn't burn")
}

fn insert_verifier(&mut self, _verifier: &Address) -> Result<()> {
Ok(())
}

/// Logging
fn log_string(&self, message: String) {
tracing::debug!("{message} for validation in IBC VP");
Expand Down
69 changes: 0 additions & 69 deletions crates/namada/src/vm/host_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::num::TryFromIntError;

use borsh::BorshDeserialize;
use borsh_ext::BorshSerializeExt;
use gas::IBC_TX_GAS;
use masp_primitives::transaction::Transaction;
use namada_core::address::ESTABLISHED_ADDRESS_BYTES_LEN;
use namada_core::arith::{self, checked};
Expand Down Expand Up @@ -79,10 +78,6 @@ pub enum TxRuntimeError {
NumConversionError(#[from] TryFromIntError),
#[error("Memory error: {0}")]
MemoryError(Box<dyn std::error::Error + Sync + Send + 'static>),
#[error("Missing tx data")]
MissingTxData,
#[error("IBC: {0}")]
Ibc(#[from] namada_ibc::Error),
#[error("No value found in result buffer")]
NoValueInResultBuffer,
#[error("VP code is not allowed in allowlist parameter.")]
Expand Down Expand Up @@ -2063,70 +2058,6 @@ where
Ok(())
}

/// Execute IBC tx.
// Temporarily the IBC tx execution is implemented via a host function to
// workaround wasm issue.
pub fn tx_ibc_execute<MEM, D, H, CA>(
env: &TxVmEnv<MEM, D, H, CA>,
) -> TxResult<i64>
where
MEM: VmMemory,
D: 'static + DB + for<'iter> DBIter<'iter>,
H: 'static + StorageHasher,
CA: WasmCacheAccess,
{
use std::rc::Rc;

use namada_ibc::{IbcActions, NftTransferModule, TransferModule};

tx_charge_gas::<MEM, D, H, CA>(env, IBC_TX_GAS)?;

let tx = unsafe { env.ctx.tx.get() };
let cmt = unsafe { env.ctx.cmt.get() };
let tx_data = tx.data(cmt).ok_or_else(|| {
let sentinel = unsafe { env.ctx.sentinel.get() };
sentinel.borrow_mut().set_invalid_commitment();
TxRuntimeError::MissingTxData
})?;
let state = Rc::new(RefCell::new(env.state()));
// Verifier set populated in tx execution
let verifiers = Rc::new(RefCell::new(BTreeSet::<Address>::new()));
// Scoped to drop `verifiers.clone`s after `actions.execute`
let transfer = {
let mut actions = IbcActions::new(state.clone(), verifiers.clone());
let module = TransferModule::new(state.clone(), verifiers.clone());
actions.add_transfer_module(module);
let module = NftTransferModule::new(state);
actions.add_transfer_module(module);
actions.execute(&tx_data)?
};
// NB: There must be no other strong references to this Rc
let verifiers = Rc::into_inner(verifiers)
.expect("There must be only one strong ref to verifiers set")
.into_inner();

// Insert all the verifiers from the tx into the verifier set in env
let verifiers_in_env = unsafe { env.ctx.verifiers.get_mut() };
for addr in verifiers.into_iter() {
tx_charge_gas::<MEM, D, H, CA>(
env,
(ESTABLISHED_ADDRESS_BYTES_LEN as u64)
.checked_mul(MEMORY_ACCESS_GAS_PER_BYTE)
.expect("Consts mul that cannot overflow"),
)?;
verifiers_in_env.insert(addr);
}

let value = transfer.serialize_to_vec();
let len: i64 = value
.len()
.try_into()
.map_err(TxRuntimeError::NumConversionError)?;
let result_buffer = unsafe { env.ctx.result_buffer.get_mut() };
result_buffer.replace(value);
Ok(len)
}

/// Validate a VP WASM code hash in a tx environment.
fn tx_validate_vp_code_hash<MEM, D, H, CA>(
env: &TxVmEnv<MEM, D, H, CA>,
Expand Down
2 changes: 1 addition & 1 deletion crates/namada/src/vm/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const UNTRUSTED_WASM_FEATURES: WasmFeatures = WasmFeatures {
relaxed_simd: false,
threads: false,
tail_call: false,
floats: false,
floats: true,
multi_memory: false,
exceptions: false,
memory64: false,
Expand Down
1 change: 0 additions & 1 deletion crates/namada/src/vm/wasm/host_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ where
"namada_tx_get_pred_epochs" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_pred_epochs),
"namada_tx_get_native_token" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_native_token),
"namada_tx_log_string" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_log_string),
"namada_tx_ibc_execute" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_ibc_execute),
"namada_tx_set_commitment_sentinel" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_set_commitment_sentinel),
"namada_tx_verify_tx_section_signature" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_verify_tx_section_signature),
"namada_tx_update_masp_note_commitment_tree" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_update_masp_note_commitment_tree),
Expand Down
22 changes: 0 additions & 22 deletions crates/tests/src/e2e/ibc_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -260,28 +260,6 @@ fn run_ledger_ibc_with_hermes() -> Result<()> {
wait_for_packet_relay(&port_id_a, &channel_id_a, &test_a)?;
check_balances_after_back(&port_id_b, &channel_id_b, &test_a, &test_b)?;

// Transfer a token and it will time out and refund
Copy link
Member Author

Choose a reason for hiding this comment

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

This removed test was duplicated

std::env::set_var(ENV_VAR_CHAIN_ID, test_b.net.chain_id.to_string());
let receiver = find_address(&test_b, BERTHA)?;
// Send a token from Chain A
transfer(
&test_a,
ALBERT,
receiver.to_string(),
NAM,
100000.0,
ALBERT_KEY,
&port_id_a,
&channel_id_a,
Some(Duration::new(0, 0)),
None,
false,
)?;
// wait for the timeout and the refund
wait_for_packet_relay(&port_id_a, &channel_id_a, &test_a)?;
// The balance should not be changed
check_balances_after_back(&port_id_b, &channel_id_b, &test_a, &test_b)?;

// Send a token to the shielded address on Chain A
transfer_on_chain(
&test_a,
Expand Down
Loading
Loading