From c224b824962adeff9f65f99d858a6a63b437ab13 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Tue, 4 Jul 2023 14:16:00 +0200 Subject: [PATCH] Add nonpayable checks Update / standardize comments --- contracts/consumer/converter/src/contract.rs | 4 +- .../simple-price-feed/src/contract.rs | 2 + .../consumer/virtual-staking/src/contract.rs | 2 +- .../provider/external-staking/src/contract.rs | 47 +++++++++++++------ .../provider/external-staking/src/crdt.rs | 5 ++ 5 files changed, 42 insertions(+), 18 deletions(-) diff --git a/contracts/consumer/converter/src/contract.rs b/contracts/consumer/converter/src/contract.rs index 56f2c564..3a1566db 100644 --- a/contracts/consumer/converter/src/contract.rs +++ b/contracts/consumer/converter/src/contract.rs @@ -112,7 +112,7 @@ impl ConverterContract<'_> { Ok(Response::new()) } - /// This is only used for tests + /// This is only used for tests. /// Ideally we want conditional compilation of these whole methods and the enum variants #[msg(exec)] fn test_stake( @@ -133,7 +133,7 @@ impl ConverterContract<'_> { } } - /// This is only used for tests + /// This is only used for tests. /// Ideally we want conditional compilation of these whole methods and the enum variants #[msg(exec)] fn test_unstake( diff --git a/contracts/consumer/simple-price-feed/src/contract.rs b/contracts/consumer/simple-price-feed/src/contract.rs index 9fe37cb7..c6edbd09 100644 --- a/contracts/consumer/simple-price-feed/src/contract.rs +++ b/contracts/consumer/simple-price-feed/src/contract.rs @@ -59,6 +59,8 @@ impl SimplePriceFeedContract<'_> { ctx: ExecCtx, native_per_foreign: Decimal, ) -> Result { + nonpayable(&ctx.info)?; + let mut config = self.config.load(ctx.deps.storage)?; config.native_per_foreign = native_per_foreign; self.config.save(ctx.deps.storage, &config)?; diff --git a/contracts/consumer/virtual-staking/src/contract.rs b/contracts/consumer/virtual-staking/src/contract.rs index 81af6ff3..ed5f8bd7 100644 --- a/contracts/consumer/virtual-staking/src/contract.rs +++ b/contracts/consumer/virtual-staking/src/contract.rs @@ -288,7 +288,7 @@ impl VirtualStakingApi for VirtualStakingContract<'_> { /// Requests to unbond tokens from a validator. This will be actually handled at the next epoch. /// If the virtual staking module is over the max cap, it will trigger a rebalance in addition to unbond. - /// If the virtual staking contract doesn't have at least amont tokens staked to the given validator, this will return an error. + /// If the virtual staking contract doesn't have at least amount tokens staked to the given validator, this will return an error. #[msg(exec)] fn unbond( &self, diff --git a/contracts/provider/external-staking/src/contract.rs b/contracts/provider/external-staking/src/contract.rs index dc819458..7f7716fe 100644 --- a/contracts/provider/external-staking/src/contract.rs +++ b/contracts/provider/external-staking/src/contract.rs @@ -4,7 +4,7 @@ use cosmwasm_std::{ }; use cw2::set_contract_version; use cw_storage_plus::{Bounder, Item, Map}; -use cw_utils::PaymentError; +use cw_utils::{nonpayable, PaymentError}; use sylvia::contract; use sylvia::types::{ExecCtx, InstantiateCtx, QueryCtx}; @@ -123,8 +123,8 @@ impl ExternalStakingContract<'_> { Ok(Response::new()) } - /// In test code, this is called from test_commit_stake. - /// In non-test code, this is called from ibc_packet_ack + /// In test code, this is called from `test_commit_stake`. + /// In non-test code, this is called from `ibc_packet_ack` pub(crate) fn commit_stake(&self, deps: DepsMut, tx_id: u64) -> Result { // Load tx let tx = self.pending_txs.load(deps.storage, tx_id)?; @@ -182,8 +182,8 @@ impl ExternalStakingContract<'_> { Ok(msg) } - /// In test code, this is called from test_rollback_stake. - /// In non-test code, this is called from ibc_packet_ack or ibc_packet_timeout + /// In test code, this is called from `test_rollback_stake`. + /// In non-test code, this is called from `ibc_packet_ack` or `ibc_packet_timeout` pub(crate) fn rollback_stake( &self, deps: DepsMut, @@ -259,7 +259,7 @@ impl ExternalStakingContract<'_> { } } - /// Rollbacks a pending stake. + /// Updates the active validator set. /// Method used for tests only. #[msg(exec)] fn test_set_active_validator( @@ -291,8 +291,8 @@ impl ExternalStakingContract<'_> { } } - /// Schedules tokens for release, adding them to the pending unbonds. After `unbonding_period` - /// passes, funds are ready to be released with `withdraw_unbonded` call by the user + /// Schedules tokens for release, adding them to the pending unbonds. After the unbonding period + /// passes, funds are ready to be released through a `withdraw_unbonded` call by the user. #[msg(exec)] pub fn unstake( &self, @@ -300,6 +300,8 @@ impl ExternalStakingContract<'_> { validator: String, amount: Coin, ) -> Result { + nonpayable(&ctx.info)?; + let config = self.config.load(ctx.deps.storage)?; ensure_eq!( @@ -369,8 +371,8 @@ impl ExternalStakingContract<'_> { Ok(resp) } - /// In test code, this is called from test_commit_unstake. - /// Method used for tests only. + /// In test code, this is called from `test_commit_unstake`. + /// In non-test code, this is called from `ibc_packet_ack` pub(crate) fn commit_unstake( &self, deps: DepsMut, @@ -442,8 +444,8 @@ impl ExternalStakingContract<'_> { Ok(()) } - /// In test code, this is called from test_rollback_unstake. - /// In non-test code, this is called from ibc_packet_ack or ibc_packet_timeout + /// In test code, this is called from `test_rollback_unstake`. + /// In non-test code, this is called from `ibc_packet_ack` or `ibc_packet_timeout` pub(crate) fn rollback_unstake(&self, deps: DepsMut, tx_id: u64) -> Result<(), ContractError> { // Load tx let tx = self.pending_txs.load(deps.storage, tx_id)?; @@ -512,10 +514,12 @@ impl ExternalStakingContract<'_> { /// Withdraws all released tokens to the sender. /// - /// Tokens to be claimed has to be unbond before by calling the `unbond` message and - /// waiting the `unbond_period` + /// Tokens to be claimed have to be unbond before by calling the `unbond` message, and + /// their unbonding period must have passed. #[msg(exec)] pub fn withdraw_unbonded(&self, ctx: ExecCtx) -> Result { + nonpayable(&ctx.info)?; + let config = self.config.load(ctx.deps.storage)?; let stake_locks: Vec<_> = self @@ -563,6 +567,8 @@ impl ExternalStakingContract<'_> { Ok(resp) } + /// Distribute rewards. + /// Method used for tests only. #[msg(exec)] pub fn test_distribute_rewards( &self, @@ -584,7 +590,8 @@ impl ExternalStakingContract<'_> { /// Distributes reward among users staking via particular validator. Distribution is performed /// proportionally to amount of tokens staked by user. - /// This is called by IBC packets in real code, but also exposed in a test only message "test_distribute_rewards" + /// In test code, this is called from `test_distribute_rewards`. + /// In non-test code, this is called from `ibc_packet_receive` pub(crate) fn distribute_rewards( &self, deps: DepsMut, @@ -632,6 +639,8 @@ impl ExternalStakingContract<'_> { /// Address on the consumer side to receive the rewards remote_recipient: String, ) -> Result { + nonpayable(&ctx.info)?; + let mut stake_lock = self .stakes .may_load(ctx.deps.storage, (&ctx.info.sender, &validator))? @@ -705,6 +714,8 @@ impl ExternalStakingContract<'_> { Ok(resp) } + /// Commits a withdraw rewards transaction. + /// Method used for tests only. #[msg(exec)] fn test_commit_withdraw_rewards( &self, @@ -723,6 +734,8 @@ impl ExternalStakingContract<'_> { } } + /// Rollbacks a withdraw rewards transaction. + /// Method used for tests only. #[msg(exec)] fn test_rollback_withdraw_rewards( &self, @@ -741,6 +754,8 @@ impl ExternalStakingContract<'_> { } } + /// In test code, this is called from `test_rollback_withdraw_rewards`. + /// In non-test code, this is called from `ibc_packet_ack` or `ibc_packet_timeout` pub(crate) fn rollback_withdraw_rewards( &self, deps: DepsMut, @@ -769,6 +784,8 @@ impl ExternalStakingContract<'_> { Ok(()) } + /// In test code, this is called from `test_commit_withdraw_rewards`. + /// In non-test code, this is called from `ibc_packet_ack` pub(crate) fn commit_withdraw_rewards( &self, deps: DepsMut, diff --git a/contracts/provider/external-staking/src/crdt.rs b/contracts/provider/external-staking/src/crdt.rs index c1ca2962..b1c43f50 100644 --- a/contracts/provider/external-staking/src/crdt.rs +++ b/contracts/provider/external-staking/src/crdt.rs @@ -59,6 +59,9 @@ impl<'a> CrdtState<'a> { } } + /// Add / Update a validator. + /// In test code, this is called from `test_set_active_validator`. + /// In non-test code, this is called from `ibc_packet_receive` pub fn add_validator( &self, storage: &mut dyn Storage, @@ -83,6 +86,8 @@ impl<'a> CrdtState<'a> { self.validators.save(storage, valoper, &state) } + /// Remove a validator. + /// In non-test code, this is called from `ibc_packet_receive` pub fn remove_validator( &self, storage: &mut dyn Storage,