Skip to content

Commit

Permalink
Removes host fn update_validity_predicate and updates wasm tests
Browse files Browse the repository at this point in the history
  • Loading branch information
grarco committed Feb 8, 2024
1 parent adb81d5 commit 1fe7b54
Show file tree
Hide file tree
Showing 9 changed files with 13 additions and 237 deletions.
54 changes: 5 additions & 49 deletions crates/namada/src/vm/host_env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,9 @@ where
tracing::debug!("tx_update {}, {:?}", key, value);

let key = Key::parse(key).map_err(TxRuntimeError::StorageDataError)?;
// FIXME: should remove this? Basically, should we allow txs to write vps to
// storage? FIXME: probably also depends if we need this for init
// account
if key.is_validity_predicate().is_some() {
tx_validate_vp_code_hash(env, &value, &None)?;
}
Expand Down Expand Up @@ -1439,55 +1442,6 @@ where
Ok(())
}

/// Update a validity predicate function exposed to the wasm VM Tx environment
pub fn tx_update_validity_predicate<MEM, DB, H, CA>(
env: &TxVmEnv<MEM, DB, H, CA>,
addr_ptr: u64,
addr_len: u64,
code_hash_ptr: u64,
code_hash_len: u64,
code_tag_ptr: u64,
code_tag_len: u64,
) -> TxResult<()>
where
MEM: VmMemory,
DB: namada_state::DB + for<'iter> namada_state::DBIter<'iter>,
H: StorageHasher,
CA: WasmCacheAccess,
{
let (addr, gas) = env
.memory
.read_string(addr_ptr, addr_len as _)
.map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?;
tx_charge_gas(env, gas)?;

let addr = Address::decode(addr).map_err(TxRuntimeError::AddressError)?;
tracing::debug!("tx_update_validity_predicate for addr {}", addr);

let (code_tag, gas) = env
.memory
.read_bytes(code_tag_ptr, code_tag_len as _)
.map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?;
tx_charge_gas(env, gas)?;
let code_tag = Option::<String>::try_from_slice(&code_tag)
.map_err(TxRuntimeError::EncodingError)?;

let key = Key::validity_predicate(&addr);
let (code_hash, gas) = env
.memory
.read_bytes(code_hash_ptr, code_hash_len as _)
.map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?;
tx_charge_gas(env, gas)?;

tx_validate_vp_code_hash(env, &code_hash, &code_tag)?;

let write_log = unsafe { env.ctx.write_log.get() };
let (gas, _size_diff) = write_log
.write(&key, code_hash)
.map_err(TxRuntimeError::StorageModificationError)?;
tx_charge_gas(env, gas)
}

/// Initialize a new account established address.
pub fn tx_init_account<MEM, DB, H, CA>(
env: &TxVmEnv<MEM, DB, H, CA>,
Expand Down Expand Up @@ -1517,6 +1471,7 @@ where
let code_tag = Option::<String>::try_from_slice(&code_tag)
.map_err(TxRuntimeError::EncodingError)?;

// FIXME: if I rmove from init maybe no need for this
tx_validate_vp_code_hash(env, &code_hash, &code_tag)?;

tracing::debug!("tx_init_account");
Expand Down Expand Up @@ -2082,6 +2037,7 @@ where
}

/// Validate a VP WASM code hash in a tx environment.
// FIXME: should remove this function?
fn tx_validate_vp_code_hash<MEM, DB, H, CA>(
env: &TxVmEnv<MEM, DB, H, CA>,
code_hash: &[u8],
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 @@ -72,7 +72,6 @@ where
"namada_tx_iter_prefix" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_iter_prefix),
"namada_tx_iter_next" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_iter_next),
"namada_tx_insert_verifier" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_insert_verifier),
"namada_tx_update_validity_predicate" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_update_validity_predicate),
"namada_tx_init_account" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_init_account),
"namada_tx_emit_ibc_event" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_emit_ibc_event),
"namada_tx_get_ibc_events" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_ibc_events),
Expand Down
35 changes: 0 additions & 35 deletions crates/tests/src/vm_host_env/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -232,41 +232,6 @@ mod tests {
tx::ctx().init_account(code_hash, &None).unwrap();
}

/// Test that a tx updating validity predicate that is not in the allowlist
/// fails.
#[test]
#[should_panic = "DisallowedVp"]
fn test_tx_update_vp_not_allowed_rejected() {
// Initialize a tx environment
tx_host_env::init();

let vp_owner = address::testing::established_address_1();
let keypair = key::testing::keypair_1();
let public_key = keypair.ref_to();
let vp_code = TestWasms::VpAlwaysTrue.read_bytes();
let vp_hash = sha256(&vp_code);

tx_host_env::with(|tx_env| {
// let mut tx_env = TestTxEnv::default();
tx_env.init_parameters(
None,
Some(vec!["some_hash".to_string()]),
None,
None,
);

// Spawn the accounts to be able to modify their storage
tx_env.spawn_accounts([&vp_owner]);
tx_env.init_account_storage(&vp_owner, vec![public_key.clone()], 1);
});

// Update VP in a transaction.
// Panics only due to unwrap in `native_host_fn!` test macro
tx::ctx()
.update_validity_predicate(&vp_owner, vp_hash, &None)
.unwrap()
}

/// Test that a tx writing validity predicate that is not in the allowlist
/// directly to storage fails
#[test]
Expand Down
8 changes: 0 additions & 8 deletions crates/tests/src/vm_host_env/tx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,14 +483,6 @@ mod native_tx_host_env {
native_host_fn!(tx_iter_prefix(prefix_ptr: u64, prefix_len: u64) -> u64);
native_host_fn!(tx_iter_next(iter_id: u64) -> i64);
native_host_fn!(tx_insert_verifier(addr_ptr: u64, addr_len: u64));
native_host_fn!(tx_update_validity_predicate(
addr_ptr: u64,
addr_len: u64,
code_hash_ptr: u64,
code_hash_len: u64,
code_tag_ptr: u64,
code_tag_len: u64,
));
native_host_fn!(tx_init_account(
code_hash_ptr: u64,
code_hash_len: u64,
Expand Down
8 changes: 0 additions & 8 deletions crates/tx_env/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,6 @@ pub trait TxEnv: StorageRead + StorageWrite {
code_tag: &Option<String>,
) -> Result<Address>;

/// Update a validity predicate
fn update_validity_predicate(
&mut self,
addr: &Address,
code: impl AsRef<[u8]>,
code_tag: &Option<String>,
) -> Result<()>;

/// Emit an IBC event. On multiple calls, these emitted event will be added.
fn emit_ibc_event(&mut self, event: &IbcEvent) -> Result<()>;

Expand Down
22 changes: 0 additions & 22 deletions crates/tx_prelude/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,28 +321,6 @@ impl TxEnv for Ctx {
.expect("Decoding address created by the ledger shouldn't fail"))
}

fn update_validity_predicate(
&mut self,
addr: &Address,
code_hash: impl AsRef<[u8]>,
code_tag: &Option<String>,
) -> Result<(), Error> {
let addr = addr.encode();
let code_hash = code_hash.as_ref();
let code_tag = code_tag.serialize_to_vec();
unsafe {
namada_tx_update_validity_predicate(
addr.as_ptr() as _,
addr.len() as _,
code_hash.as_ptr() as _,
code_hash.len() as _,
code_tag.as_ptr() as _,
code_tag.len() as _,
)
};
Ok(())
}

fn emit_ibc_event(&mut self, event: &ibc::IbcEvent) -> Result<(), Error> {
let event = borsh::to_vec(event).unwrap();
unsafe {
Expand Down
10 changes: 0 additions & 10 deletions crates/vm_env/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,6 @@ pub mod tx {
// Insert a verifier
pub fn namada_tx_insert_verifier(addr_ptr: u64, addr_len: u64);

// Update a validity predicate
pub fn namada_tx_update_validity_predicate(
addr_ptr: u64,
addr_len: u64,
code_hash_ptr: u64,
code_hash_len: u64,
code_tag_ptr: u64,
code_tag_len: u64,
);

// Initialize a new account
pub fn namada_tx_init_account(
code_hash_ptr: u64,
Expand Down
42 changes: 2 additions & 40 deletions wasm/wasm_source/src/vp_implicit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,8 @@ fn validate_tx(
verifiers: BTreeSet<Address>,
) -> VpResult {
debug_log!(
"vp_user called with user addr: {}, key_changed: {:?}, verifiers: {:?}",
"vp_implicit called with user addr: {}, key_changed: {:?}, verifiers: \
{:?}",
addr,
keys_changed,
verifiers
Expand Down Expand Up @@ -947,43 +948,4 @@ mod tests {
assert!(validate_tx(&CTX, signed_tx, vp_owner, keys_changed, verifiers).unwrap());
}
}

/// Test that a validity predicate update without a valid signature is
/// rejected.
#[test]
fn test_unsigned_vp_update_rejected() {
// Initialize a tx environment
let mut tx_env = TestTxEnv::default();

let secret_key = key::testing::keypair_1();
let public_key = secret_key.ref_to();
let vp_owner: Address = (&public_key).into();
let vp_code = TestWasms::VpAlwaysTrue.read_bytes();
let vp_hash = sha256(&vp_code);
// for the update
tx_env.store_wasm_code(vp_code);

// Spawn the accounts to be able to modify their storage
tx_env.spawn_accounts([&vp_owner]);

// Initialize VP environment from a transaction
vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| {
// Update VP in a transaction
tx::ctx()
.update_validity_predicate(address, vp_hash, &None)
.unwrap();
});

let vp_env = vp_host_env::take();
let mut tx_data = Tx::from_type(TxType::Raw);
tx_data.set_data(Data::new(vec![]));
let keys_changed: BTreeSet<storage::Key> =
vp_env.all_touched_storage_keys();
let verifiers: BTreeSet<Address> = BTreeSet::default();
vp_host_env::set(vp_env);
assert!(
!validate_tx(&CTX, tx_data, vp_owner, keys_changed, verifiers)
.unwrap()
);
}
}
70 changes: 6 additions & 64 deletions wasm/wasm_source/src/vp_user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1268,10 +1268,9 @@ mod tests {

// Initialize VP environment from a transaction
vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| {
// Update VP in a transaction
tx::ctx()
.update_validity_predicate(address, vp_hash, &None)
.unwrap();
// Update threshold in a transaction
let threshold_key = account::threshold_key(address);
tx::ctx().write(&threshold_key, 10).unwrap();
});

let vp_env = vp_host_env::take();
Expand Down Expand Up @@ -1310,66 +1309,9 @@ mod tests {

// Initialize VP environment from a transaction
vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| {
// Update VP in a transaction
tx::ctx()
.update_validity_predicate(address, vp_hash, &None)
.unwrap();
});

let pks_map = AccountPublicKeysMap::from_iter(vec![public_key]);

let mut vp_env = vp_host_env::take();
let mut tx = vp_env.tx.clone();
tx.set_data(Data::new(vec![]));
tx.set_code(Code::new(vec![], None));
tx.add_section(Section::Signature(Signature::new(
vec![tx.raw_header_hash()],
pks_map.index_secret_keys(vec![keypair]),
None,
)));
let signed_tx = tx.clone();
vp_env.tx = signed_tx.clone();
let keys_changed: BTreeSet<storage::Key> =
vp_env.all_touched_storage_keys();
let verifiers: BTreeSet<Address> = BTreeSet::default();
vp_host_env::set(vp_env);
assert!(
validate_tx(&CTX, signed_tx, vp_owner, keys_changed, verifiers)
.unwrap()
);
}

/// Test that a validity predicate update is accepted if allowed
#[test]
fn test_signed_vp_update_allowed_accepted() {
// Initialize a tx environment
let mut tx_env = TestTxEnv::default();

let vp_owner = address::testing::established_address_1();
let keypair = key::testing::keypair_1();
let public_key = keypair.ref_to();
let vp_code = TestWasms::VpAlwaysTrue.read_bytes();
let vp_hash = sha256(&vp_code);
// for the update
tx_env.store_wasm_code(vp_code);

tx_env.init_parameters(
None,
Some(vec![vp_hash.to_string()]),
None,
None,
);

// Spawn the accounts to be able to modify their storage
tx_env.spawn_accounts([&vp_owner]);
tx_env.init_account_storage(&vp_owner, vec![public_key.clone()], 1);

// Initialize VP environment from a transaction
vp_host_env::init_from_tx(vp_owner.clone(), tx_env, |address| {
// Update VP in a transaction
tx::ctx()
.update_validity_predicate(address, vp_hash, &None)
.unwrap();
// Update threshold in a transaction
let threshold_key = account::threshold_key(address);
tx::ctx().write(&threshold_key, 10).unwrap();
});

let pks_map = AccountPublicKeysMap::from_iter(vec![public_key]);
Expand Down

0 comments on commit 1fe7b54

Please sign in to comment.