From f2f9fec9235820e3ceaf7cbd97fdf605f51d2a71 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Tue, 13 Jun 2023 12:47:37 +0200 Subject: [PATCH 01/47] Update to latest sylvia v0.5.0 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index d0532741..721c28bf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,7 @@ mesh-converter = { path = "./contracts/consumer/converter" } mesh-simple-price-feed = { path = "./contracts/consumer/simple-price-feed" } mesh-virtual-staking = { path = "./contracts/consumer/virtual-staking" } -sylvia = "0.4.2" +sylvia = "0.5.0" cosmwasm-schema = "1.2" cosmwasm-std = { version = "1.2", features = ["ibc3", "cosmwasm_1_2"] } cosmwasm-storage = "1.2" From 16806f27c47567c661550babf3f1659fafeced79 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Tue, 13 Jun 2023 12:47:51 +0200 Subject: [PATCH 02/47] Update lock file --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index da820a16..4082ce3b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1017,9 +1017,9 @@ checksum = "6bdef32e8150c2a081110b42772ffe7d7c9032b606bc226c8260fd97e0976601" [[package]] name = "sylvia" -version = "0.4.2" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "653fcc60f698f37931ea686ffa3f6669b18fabead296ca69e94226cf3e1e61ce" +checksum = "66fc0aae39bc8a76742c1455e3bda09976224e31d236de0e468c91fbc2414c79" dependencies = [ "anyhow", "cosmwasm-schema", @@ -1036,9 +1036,9 @@ dependencies = [ [[package]] name = "sylvia-derive" -version = "0.4.2" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2b670d5537a6ea500e240213e100d8dddacddfc00caabcdf798347909c42a73" +checksum = "5c50a057bb2787c04dd93a203809e79d424d5de21b5024e23bc8d1cd5e6f34d4" dependencies = [ "convert_case", "proc-macro-crate", From f903fa883d44d4a36d52a532c917972f3add9af0 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Tue, 13 Jun 2023 13:04:27 +0200 Subject: [PATCH 03/47] Fix: Add missing import --- contracts/provider/native-staking/src/local_staking_api.rs | 3 +++ .../provider/native-staking/src/native_staking_callback.rs | 3 +++ 2 files changed, 6 insertions(+) diff --git a/contracts/provider/native-staking/src/local_staking_api.rs b/contracts/provider/native-staking/src/local_staking_api.rs index 80f9a28d..2df01703 100644 --- a/contracts/provider/native-staking/src/local_staking_api.rs +++ b/contracts/provider/native-staking/src/local_staking_api.rs @@ -10,6 +10,9 @@ use crate::contract::{NativeStakingContract, MAX_SLASH_PERCENTAGE, REPLY_ID_INST use crate::error::ContractError; use crate::msg::StakeMsg; +// FIXME: Move to sylvia contract macro +use crate::contract::BoundQuerier; + #[contract] #[messages(local_staking_api as LocalStakingApi)] impl LocalStakingApi for NativeStakingContract<'_> { diff --git a/contracts/provider/native-staking/src/native_staking_callback.rs b/contracts/provider/native-staking/src/native_staking_callback.rs index 1ab3ad37..40e9d21b 100644 --- a/contracts/provider/native-staking/src/native_staking_callback.rs +++ b/contracts/provider/native-staking/src/native_staking_callback.rs @@ -10,6 +10,9 @@ use mesh_native_staking_proxy::native_staking_callback::{self, NativeStakingCall use crate::contract::NativeStakingContract; use crate::error::ContractError; +// FIXME: Move to sylvia contract macro +use crate::contract::BoundQuerier; + #[contract] #[messages(native_staking_callback as NativeStakingCallback)] impl NativeStakingCallback for NativeStakingContract<'_> { From 2abbff2798f9796784df70ccb7e7bd53a4e2435c Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Tue, 13 Jun 2023 18:29:43 +0200 Subject: [PATCH 04/47] Transactional remote staking first impl --- contracts/provider/vault/src/contract.rs | 170 ++++++++++++++++++++++- contracts/provider/vault/src/error.rs | 5 +- contracts/provider/vault/src/lib.rs | 1 + contracts/provider/vault/src/txs.rs | 52 +++++++ 4 files changed, 222 insertions(+), 6 deletions(-) create mode 100644 contracts/provider/vault/src/txs.rs diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 831144c6..ad547766 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -1,6 +1,6 @@ use cosmwasm_std::{ - coin, ensure, Addr, BankMsg, Binary, Coin, Decimal, DepsMut, Order, Reply, Response, SubMsg, - SubMsgResponse, Uint128, WasmMsg, + coin, ensure, Addr, BankMsg, Binary, Coin, Decimal, DepsMut, Order, Reply, Response, StdResult, + Storage, SubMsg, SubMsgResponse, Uint128, WasmMsg, }; use cw2::set_contract_version; use cw_storage_plus::{Bounder, Item, Map}; @@ -20,6 +20,7 @@ use crate::msg::{ ConfigResponse, LienInfo, StakingInitInfo, }; use crate::state::{Config, Lien, LocalStaking, UserInfo}; +use crate::txs::{Tx, TxType, Txs}; pub const CONTRACT_NAME: &str = env!("CARGO_PKG_NAME"); pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -50,6 +51,9 @@ pub struct VaultContract<'a> { pub liens: Map<'a, (&'a Addr, &'a Addr), Lien>, /// Per-user information pub users: Map<'a, &'a Addr, UserInfo>, + /// Pending txs information + pub tx_count: Item<'a, u64>, + pub pending: Txs<'a>, } #[cfg_attr(not(feature = "library"), sylvia::entry_points)] @@ -57,15 +61,23 @@ pub struct VaultContract<'a> { #[error(ContractError)] #[messages(vault_api as VaultApi)] impl VaultContract<'_> { - pub const fn new() -> Self { + pub fn new() -> Self { Self { config: Item::new("config"), local_staking: Item::new("local_staking"), liens: Map::new("liens"), users: Map::new("users"), + pending: Txs::new("pending_txs", "users"), + tx_count: Item::new("tx_count"), } } + pub fn next_tx_id(&self, store: &mut dyn Storage) -> StdResult { + let id: u64 = self.tx_count.may_load(store)?.unwrap_or_default() + 1; + self.tx_count.save(store, &id)?; + Ok(id) + } + #[msg(instantiate)] pub fn instantiate( &self, @@ -168,7 +180,7 @@ impl VaultContract<'_> { let contract = CrossStakingApiHelper(contract); let slashable = contract.max_slash(ctx.deps.as_ref())?; - self.stake( + let tx_id = self.maybe_stake( &mut ctx, &config, &contract.0, @@ -179,6 +191,7 @@ impl VaultContract<'_> { let stake_msg = contract.receive_virtual_stake( ctx.info.sender.to_string(), amount.clone(), + // tx_id, TODO: Pass it along msg, vec![], )?; @@ -187,7 +200,8 @@ impl VaultContract<'_> { .add_message(stake_msg) .add_attribute("action", "stake_remote") .add_attribute("sender", ctx.info.sender) - .add_attribute("amount", amount.amount.to_string()); + .add_attribute("amount", amount.amount.to_string()) + .add_attribute("tx_id", tx_id.to_string()); Ok(resp) } @@ -437,6 +451,146 @@ impl VaultContract<'_> { Ok(()) } + /// Updates the pending txs for remote staking on any contract + /// + /// Stake (remote) is always called by the tokens owner, so the `sender` is + /// used as an owner address. + /// + /// Config is taken in argument as it sometimes is used outside of this function, so + /// we want to avoid double-fetching it + fn maybe_stake( + &self, + ctx: &mut ExecCtx, + config: &Config, + lienholder: &Addr, + slashable: Decimal, + amount: Coin, + ) -> Result { + ensure!( + amount.denom == config.denom, + ContractError::UnexpectedDenom(config.denom.clone()) + ); + + let amount = amount.amount; + // Tx starts here + // Verify that the user has enough collateral to stake this and the currently pending txs + let pending_amount = amount + + self + .pending + .txs + .idx + .users + .prefix(ctx.info.sender.clone()) + .range(ctx.deps.storage, None, None, Order::Ascending) + .fold(Ok(Uint128::zero()), |acc, pending| { + let acc = acc?; + pending.map(|(_, tx)| { + acc + match tx.ty { + // Value range max + TxType::Stake => tx.amount, + _ => Uint128::zero(), + } + }) + })?; + + // Load lien and update (but do not save) user info and lien holder + let lien = self + .liens + .may_load(ctx.deps.storage, (&ctx.info.sender, lienholder))? + .unwrap_or(Lien { + amount: Uint128::zero(), + slashable, + }); + // Load user and update (but do not save) max lien and total slashable + let mut user = self + .users + .may_load(ctx.deps.storage, &ctx.info.sender)? + .unwrap_or_default(); + user.max_lien = user.max_lien.max(lien.amount); + user.total_slashable += pending_amount * lien.slashable; + + ensure!(user.verify_collateral(), ContractError::InsufficentBalance); + + // Passed. Create new tx + let tx_id = self.next_tx_id(ctx.deps.storage)?; + + let new_tx = Tx { + ty: TxType::Stake, + amount, + slashable, + user: ctx.info.sender.clone(), + lienholder: lienholder.clone(), + }; + self.pending.txs.save(ctx.deps.storage, tx_id, &new_tx)?; + + Ok(tx_id) + } + + /// Commits a pending tx + // TODO: Add callback handler + #[allow(unused)] + fn commit_tx(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { + // Load tx + let tx = self.pending.txs.load(ctx.deps.storage, tx_id)?; + // TODO: Properly handle tx type + assert!(tx.ty == TxType::Stake); + // Verify tx comes from the right contract + ensure!( + tx.lienholder == ctx.info.sender, + ContractError::WrongContractTx(tx_id, ctx.info.sender.clone()) + ); + + // Load lien + let mut lien = self + .liens + .may_load(ctx.deps.storage, (&tx.user, &tx.lienholder))? + .unwrap_or(Lien { + amount: Uint128::zero(), + slashable: tx.slashable, + }); + lien.amount += tx.amount; + + let mut user = self + .users + .may_load(ctx.deps.storage, &tx.user)? + .unwrap_or_default(); + user.max_lien = user.max_lien.max(lien.amount); + user.total_slashable += tx.amount * lien.slashable; + + // FIXME: Remove, as it's a redundant check + ensure!(user.verify_collateral(), ContractError::InsufficentBalance); + + self.liens + .save(ctx.deps.storage, (&ctx.info.sender, &tx.lienholder), &lien)?; + + self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; + + // And remove tx + self.pending.txs.remove(ctx.deps.storage, tx_id)?; + + Ok(()) + } + + /// Rollbacks a pending tx + // TODO: Add callback handler + #[allow(unused)] + fn rollback_tx(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { + // Load tx + let tx = self.pending.txs.load(ctx.deps.storage, tx_id)?; + // TODO: Properly handle tx type + assert!(tx.ty == TxType::Stake); + // Verify tx comes from the right contract + ensure!( + tx.lienholder == ctx.info.sender, + ContractError::WrongContractTx(tx_id, ctx.info.sender.clone()) + ); + + // Just remove tx + self.pending.txs.remove(ctx.deps.storage, tx_id)?; + + Ok(()) + } + /// Updates the local stake for unstaking from any contract /// /// The unstake (both local and remote) is always called by the staking contract @@ -477,6 +631,12 @@ impl VaultContract<'_> { } } +impl Default for VaultContract<'_> { + fn default() -> Self { + Self::new() + } +} + #[contract] #[messages(vault_api as VaultApi)] impl VaultApi for VaultContract<'_> { diff --git a/contracts/provider/vault/src/error.rs b/contracts/provider/vault/src/error.rs index 2b176853..c213db4c 100644 --- a/contracts/provider/vault/src/error.rs +++ b/contracts/provider/vault/src/error.rs @@ -1,4 +1,4 @@ -use cosmwasm_std::{StdError, Uint128}; +use cosmwasm_std::{Addr, StdError, Uint128}; use cw_utils::{ParseReplyError, PaymentError}; use thiserror::Error; @@ -33,4 +33,7 @@ pub enum ContractError { #[error("Invalid reply id: {0}")] InvalidReplyId(u64), + + #[error("The tx {0} exists but comes from the wrong address: {1}")] + WrongContractTx(u64, Addr), } diff --git a/contracts/provider/vault/src/lib.rs b/contracts/provider/vault/src/lib.rs index 698de527..77d7a460 100644 --- a/contracts/provider/vault/src/lib.rs +++ b/contracts/provider/vault/src/lib.rs @@ -4,3 +4,4 @@ pub mod msg; #[cfg(test)] mod multitest; mod state; +pub mod txs; diff --git a/contracts/provider/vault/src/txs.rs b/contracts/provider/vault/src/txs.rs new file mode 100644 index 00000000..b3fd1e25 --- /dev/null +++ b/contracts/provider/vault/src/txs.rs @@ -0,0 +1,52 @@ +use cosmwasm_schema::cw_serde; +use cosmwasm_std::{Addr, Decimal, Uint128}; +use cw_storage_plus::{Index, IndexList, IndexedMap, MultiIndex}; + +#[cw_serde] +pub enum TxType { + Stake, + Unstake, + // TODO + // Slash, +} + +#[cw_serde] +pub struct Tx { + /// Transaction type + pub ty: TxType, + /// Associated amount + pub amount: Uint128, + /// Slashable portion of lien + pub slashable: Decimal, + /// Associated user + pub user: Addr, + /// Remote staking contract + pub lienholder: Addr, +} + +pub struct TxIndexes<'a> { + // Last type param defines the pk deserialization type + pub users: MultiIndex<'a, Addr, Tx, Addr>, +} + +impl<'a> IndexList for TxIndexes<'a> { + fn get_indexes(&'_ self) -> Box> + '_> { + let v: Vec<&dyn Index> = vec![&self.users]; + Box::new(v.into_iter()) + } +} + +pub struct Txs<'a> { + pub txs: IndexedMap<'a, u64, Tx, TxIndexes<'a>>, +} + +impl<'a> Txs<'a> { + pub fn new(storage_key: &'a str, user_subkey: &'a str) -> Self { + let indexes = TxIndexes { + users: MultiIndex::new(|_, tx| tx.user.clone(), storage_key, user_subkey), + }; + let txs = IndexedMap::new(storage_key, indexes); + + Self { txs } + } +} From 4dce85ffb99eb5925c405b2776ba53e0089ab5cc Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Thu, 15 Jun 2023 07:50:49 +0200 Subject: [PATCH 05/47] Locking approach (serialized txs) for staking Using pending txs list --- contracts/provider/vault/src/contract.rs | 41 ++++++++---------------- contracts/provider/vault/src/error.rs | 3 ++ contracts/provider/vault/src/txs.rs | 17 +++++++++- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index ad547766..eeb7101a 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -471,43 +471,30 @@ impl VaultContract<'_> { ContractError::UnexpectedDenom(config.denom.clone()) ); - let amount = amount.amount; + // Check that there are no pending txs for this user + let txs = self + .pending + .txs_by_user(ctx.deps.storage, &ctx.info.sender)?; + ensure!(txs.is_empty(), ContractError::PendingTx(txs[0].id)); + // Tx starts here - // Verify that the user has enough collateral to stake this and the currently pending txs - let pending_amount = amount - + self - .pending - .txs - .idx - .users - .prefix(ctx.info.sender.clone()) - .range(ctx.deps.storage, None, None, Order::Ascending) - .fold(Ok(Uint128::zero()), |acc, pending| { - let acc = acc?; - pending.map(|(_, tx)| { - acc + match tx.ty { - // Value range max - TxType::Stake => tx.amount, - _ => Uint128::zero(), - } - }) - })?; - - // Load lien and update (but do not save) user info and lien holder - let lien = self + let amount = amount.amount; + // Load user and update (but do not save) max lien and total slashable + let mut lien = self .liens .may_load(ctx.deps.storage, (&ctx.info.sender, lienholder))? .unwrap_or(Lien { amount: Uint128::zero(), slashable, }); - // Load user and update (but do not save) max lien and total slashable + lien.amount += amount; + let mut user = self .users .may_load(ctx.deps.storage, &ctx.info.sender)? .unwrap_or_default(); user.max_lien = user.max_lien.max(lien.amount); - user.total_slashable += pending_amount * lien.slashable; + user.total_slashable += amount * lien.slashable; ensure!(user.verify_collateral(), ContractError::InsufficentBalance); @@ -515,6 +502,7 @@ impl VaultContract<'_> { let tx_id = self.next_tx_id(ctx.deps.storage)?; let new_tx = Tx { + id: tx_id, ty: TxType::Stake, amount, slashable, @@ -557,9 +545,6 @@ impl VaultContract<'_> { user.max_lien = user.max_lien.max(lien.amount); user.total_slashable += tx.amount * lien.slashable; - // FIXME: Remove, as it's a redundant check - ensure!(user.verify_collateral(), ContractError::InsufficentBalance); - self.liens .save(ctx.deps.storage, (&ctx.info.sender, &tx.lienholder), &lien)?; diff --git a/contracts/provider/vault/src/error.rs b/contracts/provider/vault/src/error.rs index c213db4c..d249f847 100644 --- a/contracts/provider/vault/src/error.rs +++ b/contracts/provider/vault/src/error.rs @@ -34,6 +34,9 @@ pub enum ContractError { #[error("Invalid reply id: {0}")] InvalidReplyId(u64), + #[error("Transaction {0} is still pending")] + PendingTx(u64), + #[error("The tx {0} exists but comes from the wrong address: {1}")] WrongContractTx(u64, Addr), } diff --git a/contracts/provider/vault/src/txs.rs b/contracts/provider/vault/src/txs.rs index b3fd1e25..8e44c0c4 100644 --- a/contracts/provider/vault/src/txs.rs +++ b/contracts/provider/vault/src/txs.rs @@ -1,5 +1,5 @@ use cosmwasm_schema::cw_serde; -use cosmwasm_std::{Addr, Decimal, Uint128}; +use cosmwasm_std::{Addr, Decimal, Order, StdResult, Storage, Uint128}; use cw_storage_plus::{Index, IndexList, IndexedMap, MultiIndex}; #[cw_serde] @@ -12,6 +12,8 @@ pub enum TxType { #[cw_serde] pub struct Tx { + /// Transaction id + pub id: u64, /// Transaction type pub ty: TxType, /// Associated amount @@ -49,4 +51,17 @@ impl<'a> Txs<'a> { Self { txs } } + + pub fn txs_by_user(&self, storage: &dyn Storage, user: &Addr) -> StdResult> { + self.txs + .idx + .users + .prefix(user.clone()) + .range(storage, None, None, Order::Ascending) + .map(|item| { + let (_, tx) = item?; + Ok(tx) + }) + .collect::>>() + } } From 762bd3a24bc345863f5f84d8ca2560f78ba2bf0a Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Thu, 15 Jun 2023 12:10:18 +0200 Subject: [PATCH 06/47] Locking approach (serialized txs) for staking Using Lockable liens --- Cargo.lock | 1 + contracts/provider/vault/Cargo.toml | 1 + contracts/provider/vault/src/contract.rs | 142 +++++++++++++++-------- contracts/provider/vault/src/error.rs | 7 +- 4 files changed, 100 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4082ce3b..921ceda4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -697,6 +697,7 @@ dependencies = [ "cw2", "derivative", "mesh-apis", + "mesh-sync", "schemars", "serde", "sylvia", diff --git a/contracts/provider/vault/Cargo.toml b/contracts/provider/vault/Cargo.toml index 1be6e1bb..ef5b4195 100644 --- a/contracts/provider/vault/Cargo.toml +++ b/contracts/provider/vault/Cargo.toml @@ -20,6 +20,7 @@ mt = ["library", "sylvia/mt"] [dependencies] mesh-apis = { workspace = true } +mesh-sync = { workspace = true } sylvia = { workspace = true } cosmwasm-schema = { workspace = true } diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index eeb7101a..419d2b3d 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -11,6 +11,7 @@ use mesh_apis::local_staking_api::{ LocalStakingApiHelper, LocalStakingApiQueryMsg, MaxSlashResponse, }; use mesh_apis::vault_api::{self, VaultApi}; +use mesh_sync::Lockable; use sylvia::types::{ExecCtx, InstantiateCtx, QueryCtx, ReplyCtx}; use sylvia::{contract, schemars}; @@ -48,7 +49,7 @@ pub struct VaultContract<'a> { /// All liens in the protocol /// /// Liens are indexed with (user, creditor), as this pair has to be unique - pub liens: Map<'a, (&'a Addr, &'a Addr), Lien>, + pub liens: Map<'a, (&'a Addr, &'a Addr), Lockable>, /// Per-user information pub users: Map<'a, &'a Addr, UserInfo>, /// Pending txs information @@ -180,7 +181,7 @@ impl VaultContract<'_> { let contract = CrossStakingApiHelper(contract); let slashable = contract.max_slash(ctx.deps.as_ref())?; - let tx_id = self.maybe_stake( + let tx_id = self.stake_tx( &mut ctx, &config, &contract.0, @@ -288,8 +289,10 @@ impl VaultContract<'_> { let lienholder = ctx.deps.api.addr_validate(&lienholder)?; self.liens - .load(ctx.deps.storage, (&account, &lienholder)) + .load(ctx.deps.storage, (&account, &lienholder))? + .read() .map_err(Into::into) + .cloned() } /// Returns paginated claims list for an user @@ -312,8 +315,10 @@ impl VaultContract<'_> { .liens .prefix(&account) .range(ctx.deps.storage, bound, None, Order::Ascending) - .map(|lien| { - lien.map(|(lienholder, lien)| LienInfo { + .map(|item| { + let (lienholder, lien) = item?; + let lien = lien.read()?; + Ok::(LienInfo { lienholder: lienholder.into(), amount: lien.amount, }) @@ -428,10 +433,11 @@ impl VaultContract<'_> { let mut lien = self .liens .may_load(ctx.deps.storage, (&ctx.info.sender, lienholder))? - .unwrap_or(Lien { + .unwrap_or(Lockable::new(Lien { amount: Uint128::zero(), slashable, - }); + })); + let lien = lien.write()?; lien.amount += amount; let mut user = self @@ -443,8 +449,11 @@ impl VaultContract<'_> { ensure!(user.verify_collateral(), ContractError::InsufficentBalance); - self.liens - .save(ctx.deps.storage, (&ctx.info.sender, lienholder), &lien)?; + self.liens.save( + ctx.deps.storage, + (&ctx.info.sender, lienholder), + &Lockable::new(lien.clone()), + )?; self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; @@ -458,7 +467,7 @@ impl VaultContract<'_> { /// /// Config is taken in argument as it sometimes is used outside of this function, so /// we want to avoid double-fetching it - fn maybe_stake( + fn stake_tx( &self, ctx: &mut ExecCtx, config: &Config, @@ -471,24 +480,21 @@ impl VaultContract<'_> { ContractError::UnexpectedDenom(config.denom.clone()) ); - // Check that there are no pending txs for this user - let txs = self - .pending - .txs_by_user(ctx.deps.storage, &ctx.info.sender)?; - ensure!(txs.is_empty(), ContractError::PendingTx(txs[0].id)); - // Tx starts here let amount = amount.amount; - // Load user and update (but do not save) max lien and total slashable - let mut lien = self + // Load user and update max lien and total slashable + // Write lock lien + let mut lien_lock = self .liens .may_load(ctx.deps.storage, (&ctx.info.sender, lienholder))? - .unwrap_or(Lien { + .unwrap_or(Lockable::new(Lien { amount: Uint128::zero(), slashable, - }); + })); + let lien = lien_lock.write()?; lien.amount += amount; + // TODO?: Lockable let mut user = self .users .may_load(ctx.deps.storage, &ctx.info.sender)? @@ -498,7 +504,14 @@ impl VaultContract<'_> { ensure!(user.verify_collateral(), ContractError::InsufficentBalance); - // Passed. Create new tx + // Write lock it + lien_lock.lock_write()?; + self.liens + .save(ctx.deps.storage, (&ctx.info.sender, lienholder), &lien_lock)?; + + self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; + + // Create new tx let tx_id = self.next_tx_id(ctx.deps.storage)?; let new_tx = Tx { @@ -529,28 +542,19 @@ impl VaultContract<'_> { ); // Load lien - let mut lien = self + let mut lien_lock = self .liens - .may_load(ctx.deps.storage, (&tx.user, &tx.lienholder))? - .unwrap_or(Lien { - amount: Uint128::zero(), - slashable: tx.slashable, - }); - lien.amount += tx.amount; - - let mut user = self - .users - .may_load(ctx.deps.storage, &tx.user)? - .unwrap_or_default(); - user.max_lien = user.max_lien.max(lien.amount); - user.total_slashable += tx.amount * lien.slashable; - - self.liens - .save(ctx.deps.storage, (&ctx.info.sender, &tx.lienholder), &lien)?; - - self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; + .load(ctx.deps.storage, (&tx.user, &tx.lienholder))?; + // Unlock it + lien_lock.unlock_write()?; + // Save it + self.liens.save( + ctx.deps.storage, + (&ctx.info.sender, &tx.lienholder), + &lien_lock, + )?; - // And remove tx + // Remove tx self.pending.txs.remove(ctx.deps.storage, tx_id)?; Ok(()) @@ -570,7 +574,43 @@ impl VaultContract<'_> { ContractError::WrongContractTx(tx_id, ctx.info.sender.clone()) ); - // Just remove tx + // Load lien + let mut lien_lock = self + .liens + .load(ctx.deps.storage, (&tx.user, &tx.lienholder))?; + // Rollback amount (need to unlock it first) + lien_lock.unlock_write()?; + let lien = lien_lock.write()?; + lien.amount -= tx.amount; + // Unlock lien + lien_lock.unlock_write()?; + // Save it unlocked + self.liens.save( + ctx.deps.storage, + (&ctx.info.sender, &tx.lienholder), + &lien_lock, + )?; + + // Rollback user's max_lien + let mut user = self.users.load(ctx.deps.storage, &tx.user)?; + + // Max lien has to be recalculated from scratch; the just rolled back lien + // is already written to storage + user.max_lien = self + .liens + .prefix(&tx.user) + .range(ctx.deps.storage, None, None, Order::Ascending) + .try_fold(Uint128::zero(), |max_lien, item| { + let (_, lien_lock) = item?; + // Shouldn't fail, because unlocked already + let lien = lien_lock.read().map_err(Into::::into); + lien.map(|lien| max_lien.max(lien.amount)) + })?; + + user.total_slashable -= tx.amount * tx.slashable; + self.users.save(ctx.deps.storage, &tx.user, &user)?; + + // Remove tx self.pending.txs.remove(ctx.deps.storage, tx_id)?; Ok(()) @@ -586,30 +626,36 @@ impl VaultContract<'_> { let amount = amount.amount; let owner = Addr::unchecked(owner); - let mut lien = self + // TODO: Txs + let mut lien_lock = self .liens .may_load(ctx.deps.storage, (&owner, &ctx.info.sender))? .ok_or(ContractError::UnknownLienholder)?; + let lien = lien_lock.write()?; ensure!(lien.amount >= amount, ContractError::InsufficientLien); + let slashable = lien.slashable; lien.amount -= amount; self.liens - .save(ctx.deps.storage, (&owner, &ctx.info.sender), &lien)?; + .save(ctx.deps.storage, (&owner, &ctx.info.sender), &lien_lock)?; let mut user = self.users.load(ctx.deps.storage, &owner)?; - // Max lien has to be recalculated from scratch; the just released lien + // Max lien has to be recalculated from scratch; the just saved lien // is already written to storage user.max_lien = self .liens .prefix(&owner) .range(ctx.deps.storage, None, None, Order::Ascending) - .try_fold(Uint128::zero(), |max_lien, lien| { - lien.map(|(_, lien)| max_lien.max(lien.amount)) + .try_fold(Uint128::zero(), |max_lien, item| { + let (_, lien_lock) = item?; + // FIXME: Fails as write locked + let lien = lien_lock.read().map_err(Into::::into); + lien.map(|lien| max_lien.max(lien.amount)) })?; - user.total_slashable -= amount * lien.slashable; + user.total_slashable -= amount * slashable; self.users.save(ctx.deps.storage, &owner, &user)?; Ok(()) diff --git a/contracts/provider/vault/src/error.rs b/contracts/provider/vault/src/error.rs index d249f847..5f56b09f 100644 --- a/contracts/provider/vault/src/error.rs +++ b/contracts/provider/vault/src/error.rs @@ -1,5 +1,6 @@ use cosmwasm_std::{Addr, StdError, Uint128}; use cw_utils::{ParseReplyError, PaymentError}; +use mesh_sync::LockError; use thiserror::Error; #[derive(Error, Debug, PartialEq)] @@ -13,6 +14,9 @@ pub enum ContractError { #[error("{0}")] ParseReply(#[from] ParseReplyError), + #[error("{0}")] + Lock(#[from] LockError), + #[error("Unauthorized")] Unauthorized {}, @@ -34,9 +38,6 @@ pub enum ContractError { #[error("Invalid reply id: {0}")] InvalidReplyId(u64), - #[error("Transaction {0} is still pending")] - PendingTx(u64), - #[error("The tx {0} exists but comes from the wrong address: {1}")] WrongContractTx(u64, Addr), } From fdf33c90bccf81ed6aca94cab92940fdce878adc Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Thu, 15 Jun 2023 16:05:03 +0200 Subject: [PATCH 07/47] Add tx id to cross staking api / msg --- contracts/provider/external-staking/src/contract.rs | 1 + contracts/provider/external-staking/src/msg.rs | 2 ++ contracts/provider/external-staking/src/multitest.rs | 12 ++++++++++++ contracts/provider/vault/src/contract.rs | 2 +- contracts/provider/vault/src/multitest.rs | 2 +- .../provider/vault/src/multitest/cross_staking.rs | 1 + packages/apis/src/cross_staking_api.rs | 9 ++++++++- packages/mocks/src/cross_staking.rs | 3 ++- packages/mocks/src/vault.rs | 3 +++ 9 files changed, 31 insertions(+), 4 deletions(-) diff --git a/contracts/provider/external-staking/src/contract.rs b/contracts/provider/external-staking/src/contract.rs index c837e23f..c439ed49 100644 --- a/contracts/provider/external-staking/src/contract.rs +++ b/contracts/provider/external-staking/src/contract.rs @@ -391,6 +391,7 @@ pub mod cross_staking { ctx: ExecCtx, owner: String, amount: Coin, + _tx_id: u64, msg: Binary, ) -> Result { let config = self.config.load(ctx.deps.storage)?; diff --git a/contracts/provider/external-staking/src/msg.rs b/contracts/provider/external-staking/src/msg.rs index 08d30f9f..2bdb1737 100644 --- a/contracts/provider/external-staking/src/msg.rs +++ b/contracts/provider/external-staking/src/msg.rs @@ -39,6 +39,8 @@ pub struct StakesResponse { /// Message to be send as `msg` field on `receive_virtual_staking` #[cw_serde] pub struct ReceiveVirtualStake { + /// Associated transaction id + pub tx: u64, pub validator: String, } diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index 9d73e20f..3f110cd1 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -120,6 +120,7 @@ fn staking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { + tx: 1, validator: validators[0].to_string(), }) .unwrap(), @@ -132,6 +133,7 @@ fn staking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { + tx: 2, validator: validators[1].to_string(), }) .unwrap(), @@ -144,6 +146,7 @@ fn staking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { + tx: 3, validator: validators[0].to_string(), }) .unwrap(), @@ -156,6 +159,7 @@ fn staking() { contract.contract_addr.to_string(), coin(200, OSMO), to_binary(&ReceiveVirtualStake { + tx: 4, validator: validators[1].to_string(), }) .unwrap(), @@ -168,6 +172,7 @@ fn staking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { + tx: 5, validator: validators[0].to_string(), }) .unwrap(), @@ -294,6 +299,7 @@ fn unstaking() { contract.contract_addr.to_string(), coin(200, OSMO), to_binary(&ReceiveVirtualStake { + tx: 1, validator: validators[0].to_string(), }) .unwrap(), @@ -306,6 +312,7 @@ fn unstaking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { + tx: 2, validator: validators[1].to_string(), }) .unwrap(), @@ -318,6 +325,7 @@ fn unstaking() { contract.contract_addr.to_string(), coin(300, OSMO), to_binary(&ReceiveVirtualStake { + tx: 3, validator: validators[0].to_string(), }) .unwrap(), @@ -577,6 +585,7 @@ fn distribution() { contract.contract_addr.to_string(), coin(200, OSMO), to_binary(&ReceiveVirtualStake { + tx: 1, validator: validators[0].to_string(), }) .unwrap(), @@ -589,6 +598,7 @@ fn distribution() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { + tx: 2, validator: validators[1].to_string(), }) .unwrap(), @@ -601,6 +611,7 @@ fn distribution() { contract.contract_addr.to_string(), coin(300, OSMO), to_binary(&ReceiveVirtualStake { + tx: 3, validator: validators[0].to_string(), }) .unwrap(), @@ -818,6 +829,7 @@ fn distribution() { contract.contract_addr.to_string(), coin(300, OSMO), to_binary(&ReceiveVirtualStake { + tx: 2, validator: validators[1].to_string(), }) .unwrap(), diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 419d2b3d..6e543ea6 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -192,7 +192,7 @@ impl VaultContract<'_> { let stake_msg = contract.receive_virtual_stake( ctx.info.sender.to_string(), amount.clone(), - // tx_id, TODO: Pass it along + tx_id, msg, vec![], )?; diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index 51127325..44b54df1 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -511,7 +511,7 @@ fn stake_cross() { coin(0, OSMO) ); - // Staking localy + // Staking remotely vault .stake_remote( diff --git a/contracts/provider/vault/src/multitest/cross_staking.rs b/contracts/provider/vault/src/multitest/cross_staking.rs index 906802e8..713785c5 100644 --- a/contracts/provider/vault/src/multitest/cross_staking.rs +++ b/contracts/provider/vault/src/multitest/cross_staking.rs @@ -66,6 +66,7 @@ impl CrossStakingApi for CrossStaking<'_> { _ctx: ExecCtx, _owner: String, _amount: Coin, + _tx: u64, _msg: Binary, ) -> Result { Ok(Response::new()) diff --git a/packages/apis/src/cross_staking_api.rs b/packages/apis/src/cross_staking_api.rs index 4a4db44b..cb3129f7 100644 --- a/packages/apis/src/cross_staking_api.rs +++ b/packages/apis/src/cross_staking_api.rs @@ -24,6 +24,7 @@ pub trait CrossStakingApi { ctx: ExecCtx, owner: String, amount: Coin, + tx_id: u64, msg: Binary, ) -> Result; @@ -44,10 +45,16 @@ impl CrossStakingApiHelper { &self, owner: String, amount: Coin, + tx_id: u64, msg: Binary, funds: Vec, ) -> Result { - let msg = CrossStakingApiExecMsg::ReceiveVirtualStake { owner, msg, amount }; + let msg = CrossStakingApiExecMsg::ReceiveVirtualStake { + owner, + msg, + amount, + tx_id, + }; let wasm = WasmMsg::Execute { contract_addr: self.0.to_string(), msg: to_binary(&msg)?, diff --git a/packages/mocks/src/cross_staking.rs b/packages/mocks/src/cross_staking.rs index 9b268871..2053d6a8 100644 --- a/packages/mocks/src/cross_staking.rs +++ b/packages/mocks/src/cross_staking.rs @@ -101,6 +101,7 @@ impl CrossStakingApi for MockCrossStakingContract<'_> { ctx: ExecCtx, owner: String, amount: Coin, + tx_id: u64, msg: Binary, ) -> Result { nonpayable(&ctx.info)?; @@ -120,7 +121,7 @@ impl CrossStakingApi for MockCrossStakingContract<'_> { ); // ignore args - let _ = (owner, msg, amount); + let _ = (owner, msg, amount, tx_id); Ok(Response::new()) } diff --git a/packages/mocks/src/vault.rs b/packages/mocks/src/vault.rs index ee334928..eb331027 100644 --- a/packages/mocks/src/vault.rs +++ b/packages/mocks/src/vault.rs @@ -129,6 +129,8 @@ impl MockVaultContract<'_> { contract: String, // amount to stake on that contract amount: Coin, + // associated transaction id + tx_id: u64, // action to take with that stake msg: Binary, ) -> Result { @@ -139,6 +141,7 @@ impl MockVaultContract<'_> { let wasm_msg = cross_staking.receive_virtual_stake( ctx.info.sender.into_string(), amount, + tx_id, msg, vec![], )?; From 8dcc0ce36f01d1cf0c91f88ada319f830ad738fc Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Thu, 15 Jun 2023 17:03:25 +0200 Subject: [PATCH 08/47] Fix: remove double unlock write --- contracts/provider/vault/src/contract.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 6e543ea6..310cf0c7 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -582,8 +582,6 @@ impl VaultContract<'_> { lien_lock.unlock_write()?; let lien = lien_lock.write()?; lien.amount -= tx.amount; - // Unlock lien - lien_lock.unlock_write()?; // Save it unlocked self.liens.save( ctx.deps.storage, From cf34bcadfe2e8a2682d4458a8557a7bf788f2e54 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 07:47:51 +0200 Subject: [PATCH 09/47] Unify local and remote stake --- contracts/provider/vault/src/contract.rs | 111 ++++++++--------------- 1 file changed, 40 insertions(+), 71 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 310cf0c7..53e297ad 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -181,12 +181,13 @@ impl VaultContract<'_> { let contract = CrossStakingApiHelper(contract); let slashable = contract.max_slash(ctx.deps.as_ref())?; - let tx_id = self.stake_tx( + let tx_id = self.stake( &mut ctx, &config, &contract.0, slashable.max_slash, amount.clone(), + true, )?; let stake_msg = contract.receive_virtual_stake( @@ -228,6 +229,7 @@ impl VaultContract<'_> { &local_staking.contract.0, local_staking.max_slash, amount.clone(), + false, )?; let stake_msg = local_staking.contract.receive_stake( @@ -423,67 +425,14 @@ impl VaultContract<'_> { lienholder: &Addr, slashable: Decimal, amount: Coin, - ) -> Result<(), ContractError> { - ensure!( - amount.denom == config.denom, - ContractError::UnexpectedDenom(config.denom.clone()) - ); - - let amount = amount.amount; - let mut lien = self - .liens - .may_load(ctx.deps.storage, (&ctx.info.sender, lienholder))? - .unwrap_or(Lockable::new(Lien { - amount: Uint128::zero(), - slashable, - })); - let lien = lien.write()?; - lien.amount += amount; - - let mut user = self - .users - .may_load(ctx.deps.storage, &ctx.info.sender)? - .unwrap_or_default(); - user.max_lien = user.max_lien.max(lien.amount); - user.total_slashable += amount * lien.slashable; - - ensure!(user.verify_collateral(), ContractError::InsufficentBalance); - - self.liens.save( - ctx.deps.storage, - (&ctx.info.sender, lienholder), - &Lockable::new(lien.clone()), - )?; - - self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; - - Ok(()) - } - - /// Updates the pending txs for remote staking on any contract - /// - /// Stake (remote) is always called by the tokens owner, so the `sender` is - /// used as an owner address. - /// - /// Config is taken in argument as it sometimes is used outside of this function, so - /// we want to avoid double-fetching it - fn stake_tx( - &self, - ctx: &mut ExecCtx, - config: &Config, - lienholder: &Addr, - slashable: Decimal, - amount: Coin, + remote: bool, ) -> Result { ensure!( amount.denom == config.denom, ContractError::UnexpectedDenom(config.denom.clone()) ); - // Tx starts here let amount = amount.amount; - // Load user and update max lien and total slashable - // Write lock lien let mut lien_lock = self .liens .may_load(ctx.deps.storage, (&ctx.info.sender, lienholder))? @@ -494,7 +443,7 @@ impl VaultContract<'_> { let lien = lien_lock.write()?; lien.amount += amount; - // TODO?: Lockable + // TODO: Lockable let mut user = self .users .may_load(ctx.deps.storage, &ctx.info.sender)? @@ -504,26 +453,51 @@ impl VaultContract<'_> { ensure!(user.verify_collateral(), ContractError::InsufficentBalance); - // Write lock it - lien_lock.lock_write()?; + // Write lock it if remote stake + if remote { + lien_lock.lock_write()?; + } self.liens .save(ctx.deps.storage, (&ctx.info.sender, lienholder), &lien_lock)?; self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; - // Create new tx - let tx_id = self.next_tx_id(ctx.deps.storage)?; + if remote { + // Create new tx + let tx_id = self.new_tx( + ctx.deps.storage, + TxType::Stake, + amount, + &ctx.info.sender, + lienholder, + slashable, + )?; + Ok(tx_id) + } else { + Ok(0) + } + } + + fn new_tx( + &self, + storage: &mut dyn Storage, + ty: TxType, + amount: Uint128, + owner: &Addr, + holder: &Addr, + slashable: Decimal, + ) -> Result { + let tx_id = self.next_tx_id(storage)?; let new_tx = Tx { id: tx_id, - ty: TxType::Stake, + ty, amount, slashable, - user: ctx.info.sender.clone(), - lienholder: lienholder.clone(), + user: owner.clone(), + lienholder: holder.clone(), }; - self.pending.txs.save(ctx.deps.storage, tx_id, &new_tx)?; - + self.pending.txs.save(storage, tx_id, &new_tx)?; Ok(tx_id) } @@ -533,8 +507,7 @@ impl VaultContract<'_> { fn commit_tx(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { // Load tx let tx = self.pending.txs.load(ctx.deps.storage, tx_id)?; - // TODO: Properly handle tx type - assert!(tx.ty == TxType::Stake); + // Verify tx comes from the right contract ensure!( tx.lienholder == ctx.info.sender, @@ -566,8 +539,6 @@ impl VaultContract<'_> { fn rollback_tx(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { // Load tx let tx = self.pending.txs.load(ctx.deps.storage, tx_id)?; - // TODO: Properly handle tx type - assert!(tx.ty == TxType::Stake); // Verify tx comes from the right contract ensure!( tx.lienholder == ctx.info.sender, @@ -610,7 +581,6 @@ impl VaultContract<'_> { // Remove tx self.pending.txs.remove(ctx.deps.storage, tx_id)?; - Ok(()) } @@ -648,7 +618,6 @@ impl VaultContract<'_> { .range(ctx.deps.storage, None, None, Order::Ascending) .try_fold(Uint128::zero(), |max_lien, item| { let (_, lien_lock) = item?; - // FIXME: Fails as write locked let lien = lien_lock.read().map_err(Into::::into); lien.map(|lien| max_lien.max(lien.amount)) })?; From 338629e79e60a59f02022fd92bac3e3323559df0 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 08:23:12 +0200 Subject: [PATCH 10/47] Lock user info as well --- contracts/provider/vault/src/contract.rs | 94 ++++++++++++------------ 1 file changed, 47 insertions(+), 47 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 53e297ad..cfdc5bfd 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -51,7 +51,7 @@ pub struct VaultContract<'a> { /// Liens are indexed with (user, creditor), as this pair has to be unique pub liens: Map<'a, (&'a Addr, &'a Addr), Lockable>, /// Per-user information - pub users: Map<'a, &'a Addr, UserInfo>, + pub users: Map<'a, &'a Addr, Lockable>, /// Pending txs information pub tx_count: Item<'a, u64>, pub pending: Txs<'a>, @@ -111,12 +111,14 @@ impl VaultContract<'_> { let denom = self.config.load(ctx.deps.storage)?.denom; let amount = must_pay(&ctx.info, &denom)?; - let mut user = self + let mut user_lock = self .users .may_load(ctx.deps.storage, &ctx.info.sender)? .unwrap_or_default(); + let user = user_lock.write()?; user.collateral += amount; - self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; + self.users + .save(ctx.deps.storage, &ctx.info.sender, &user_lock)?; let resp = Response::new() .add_attribute("action", "bond") @@ -134,10 +136,11 @@ impl VaultContract<'_> { ensure!(denom == amount.denom, ContractError::UnexpectedDenom(denom)); - let mut user = self + let mut user_lock = self .users .may_load(ctx.deps.storage, &ctx.info.sender)? .unwrap_or_default(); + let user = user_lock.read()?; let free_collateral = user.free_collateral(); ensure!( @@ -145,8 +148,10 @@ impl VaultContract<'_> { ContractError::ClaimsLocked(free_collateral) ); + let user = user_lock.write()?; user.collateral -= amount.amount; - self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; + self.users + .save(ctx.deps.storage, &ctx.info.sender, &user_lock)?; let msg = BankMsg::Send { to_address: ctx.info.sender.to_string(), @@ -252,10 +257,11 @@ impl VaultContract<'_> { let denom = self.config.load(ctx.deps.storage)?.denom; let account = ctx.deps.api.addr_validate(&account)?; - let user = self + let user_lock = self .users .may_load(ctx.deps.storage, &account)? .unwrap_or_default(); + let user = user_lock.read()?; let resp = AccountResponse { denom, @@ -360,11 +366,19 @@ impl VaultContract<'_> { !(with_collateral && account .as_ref() - .map(|(_, account)| account.collateral.is_zero()) + .map(|(_, account_lock)| { + // FIXME: This skips write-locked accounts + match account_lock.read() { + Ok(account) => account.collateral.is_zero(), + Err(_) => false, + } + }) .unwrap_or(false)) }) .map(|account| { - account.map(|(addr, account)| AllAccountsResponseItem { + let (addr, account_lock) = account?; + let account = account_lock.read()?; + Ok::(AllAccountsResponseItem { account: addr.into(), denom: denom.clone(), bonded: account.collateral, @@ -443,11 +457,11 @@ impl VaultContract<'_> { let lien = lien_lock.write()?; lien.amount += amount; - // TODO: Lockable - let mut user = self + let mut user_lock = self .users .may_load(ctx.deps.storage, &ctx.info.sender)? .unwrap_or_default(); + let mut user = user_lock.write()?; user.max_lien = user.max_lien.max(lien.amount); user.total_slashable += amount * lien.slashable; @@ -456,51 +470,33 @@ impl VaultContract<'_> { // Write lock it if remote stake if remote { lien_lock.lock_write()?; + user_lock.lock_write()?; } self.liens .save(ctx.deps.storage, (&ctx.info.sender, lienholder), &lien_lock)?; - self.users.save(ctx.deps.storage, &ctx.info.sender, &user)?; + self.users + .save(ctx.deps.storage, &ctx.info.sender, &user_lock)?; if remote { // Create new tx - let tx_id = self.new_tx( - ctx.deps.storage, - TxType::Stake, + let tx_id = self.next_tx_id(ctx.deps.storage)?; + + let new_tx = Tx { + id: tx_id, + ty: TxType::Stake, amount, - &ctx.info.sender, - lienholder, slashable, - )?; + user: ctx.info.sender.clone(), + lienholder: lienholder.clone(), + }; + self.pending.txs.save(ctx.deps.storage, tx_id, &new_tx)?; Ok(tx_id) } else { Ok(0) } } - fn new_tx( - &self, - storage: &mut dyn Storage, - ty: TxType, - amount: Uint128, - owner: &Addr, - holder: &Addr, - slashable: Decimal, - ) -> Result { - let tx_id = self.next_tx_id(storage)?; - - let new_tx = Tx { - id: tx_id, - ty, - amount, - slashable, - user: owner.clone(), - lienholder: holder.clone(), - }; - self.pending.txs.save(storage, tx_id, &new_tx)?; - Ok(tx_id) - } - /// Commits a pending tx // TODO: Add callback handler #[allow(unused)] @@ -560,8 +556,11 @@ impl VaultContract<'_> { &lien_lock, )?; - // Rollback user's max_lien - let mut user = self.users.load(ctx.deps.storage, &tx.user)?; + // Load user + let mut user_lock = self.users.load(ctx.deps.storage, &tx.user)?; + // Rollback user's max_lien (need to unlock it first) + lien_lock.unlock_write()?; + let mut user = user_lock.write()?; // Max lien has to be recalculated from scratch; the just rolled back lien // is already written to storage @@ -577,7 +576,7 @@ impl VaultContract<'_> { })?; user.total_slashable -= tx.amount * tx.slashable; - self.users.save(ctx.deps.storage, &tx.user, &user)?; + self.users.save(ctx.deps.storage, &tx.user, &user_lock)?; // Remove tx self.pending.txs.remove(ctx.deps.storage, tx_id)?; @@ -594,21 +593,22 @@ impl VaultContract<'_> { let amount = amount.amount; let owner = Addr::unchecked(owner); - // TODO: Txs let mut lien_lock = self .liens .may_load(ctx.deps.storage, (&owner, &ctx.info.sender))? .ok_or(ContractError::UnknownLienholder)?; - let lien = lien_lock.write()?; + let lien = lien_lock.read()?; ensure!(lien.amount >= amount, ContractError::InsufficientLien); let slashable = lien.slashable; + let lien = lien_lock.write()?; lien.amount -= amount; self.liens .save(ctx.deps.storage, (&owner, &ctx.info.sender), &lien_lock)?; - let mut user = self.users.load(ctx.deps.storage, &owner)?; + let mut user_lock = self.users.load(ctx.deps.storage, &owner)?; + let mut user = user_lock.write()?; // Max lien has to be recalculated from scratch; the just saved lien // is already written to storage @@ -623,7 +623,7 @@ impl VaultContract<'_> { })?; user.total_slashable -= amount * slashable; - self.users.save(ctx.deps.storage, &owner, &user)?; + self.users.save(ctx.deps.storage, &owner, &user_lock)?; Ok(()) } From 6ea02a59931e7f87a2ab6a5bd32c4d3f57d594cb Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 09:20:56 +0200 Subject: [PATCH 11/47] Add commit / rollback tx to vault api --- .../provider/external-staking/src/contract.rs | 3 ++- packages/apis/src/vault_api.rs | 10 ++++++++++ packages/mocks/src/vault.rs | 16 ++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/contracts/provider/external-staking/src/contract.rs b/contracts/provider/external-staking/src/contract.rs index c439ed49..5146f472 100644 --- a/contracts/provider/external-staking/src/contract.rs +++ b/contracts/provider/external-staking/src/contract.rs @@ -435,7 +435,8 @@ pub mod cross_staking { let resp = Response::new() .add_attribute("action", "receive_virtual_stake") .add_attribute("owner", owner) - .add_attribute("amount", amount.amount.to_string()); + .add_attribute("amount", amount.amount.to_string()) + .add_attribute("tx_id", msg.tx.to_string()); Ok(resp) } diff --git a/packages/apis/src/vault_api.rs b/packages/apis/src/vault_api.rs index 5ca50517..541cfe85 100644 --- a/packages/apis/src/vault_api.rs +++ b/packages/apis/src/vault_api.rs @@ -29,6 +29,16 @@ pub trait VaultApi { // address of the user who originally called stake_remote owner: String, ) -> Result; + + /// This must be called by the remote staking contract to commit the remote staking call on success. + /// Transaction ID is used to identify the original (vault contract originated) transaction. + #[msg(exec)] + fn commit_tx(&self, ctx: ExecCtx, tx_id: u64) -> Result; + + /// This must be called by the remote staking contract to rollback the remote staking call on failure. + /// Transaction ID is used to identify the original (vault contract originated) transaction. + #[msg(exec)] + fn rollback_tx(&self, ctx: ExecCtx, tx_id: u64) -> Result; } #[cw_serde] diff --git a/packages/mocks/src/vault.rs b/packages/mocks/src/vault.rs index eb331027..7e207b84 100644 --- a/packages/mocks/src/vault.rs +++ b/packages/mocks/src/vault.rs @@ -229,4 +229,20 @@ impl VaultApi for MockVaultContract<'_> { // we don't track liens so no-op Ok(Response::new()) } + + /// This must be called by the remote staking contract to commit the remote staking call on success. + /// Transaction ID is used to identify the original (vault contract originated) transaction. + #[msg(exec)] + fn commit_tx(&self, _ctx: ExecCtx, tx_id: u64) -> Result { + let _ = tx_id; + Ok(Response::new()) + } + + /// This must be called by the remote staking contract to rollback the remote staking call on failure. + /// Transaction ID is used to identify the original (vault contract originated) transaction. + #[msg(exec)] + fn rollback_tx(&self, _ctx: ExecCtx, tx_id: u64) -> Result { + let _ = tx_id; + Ok(Response::new()) + } } From e5a24e03cbfd14ef1e943a00eb182212e9386442 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 09:23:51 +0200 Subject: [PATCH 12/47] Commit / rollback tx harnessing --- contracts/provider/vault/src/contract.rs | 35 ++++++++++++++++++------ contracts/provider/vault/src/txs.rs | 3 +- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index cfdc5bfd..ab022063 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -484,7 +484,7 @@ impl VaultContract<'_> { let new_tx = Tx { id: tx_id, - ty: TxType::Stake, + ty: TxType::InFlightStaking, amount, slashable, user: ctx.info.sender.clone(), @@ -497,10 +497,8 @@ impl VaultContract<'_> { } } - /// Commits a pending tx - // TODO: Add callback handler - #[allow(unused)] - fn commit_tx(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { + /// Commits a pending stake + fn commit_stake(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { // Load tx let tx = self.pending.txs.load(ctx.deps.storage, tx_id)?; @@ -530,9 +528,7 @@ impl VaultContract<'_> { } /// Rollbacks a pending tx - // TODO: Add callback handler - #[allow(unused)] - fn rollback_tx(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { + fn rollback_stake(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { // Load tx let tx = self.pending.txs.load(ctx.deps.storage, tx_id)?; // Verify tx comes from the right contract @@ -685,4 +681,27 @@ impl VaultApi for VaultContract<'_> { Ok(resp) } + + #[msg(exec)] + fn commit_tx(&self, mut ctx: ExecCtx, tx_id: u64) -> Result { + self.commit_stake(&mut ctx, tx_id)?; + + let resp = Response::new() + .add_attribute("action", "commit_tx") + .add_attribute("sender", ctx.info.sender) + .add_attribute("tx_id", tx_id.to_string()); + + Ok(resp) + } + + #[msg(exec)] + fn rollback_tx(&self, mut ctx: ExecCtx, tx_id: u64) -> Result { + self.rollback_stake(&mut ctx, tx_id)?; + + let resp = Response::new() + .add_attribute("action", "rollback_tx") + .add_attribute("sender", ctx.info.sender) + .add_attribute("tx_id", tx_id.to_string()); + Ok(resp) + } } diff --git a/contracts/provider/vault/src/txs.rs b/contracts/provider/vault/src/txs.rs index 8e44c0c4..734330cf 100644 --- a/contracts/provider/vault/src/txs.rs +++ b/contracts/provider/vault/src/txs.rs @@ -4,8 +4,7 @@ use cw_storage_plus::{Index, IndexList, IndexedMap, MultiIndex}; #[cw_serde] pub enum TxType { - Stake, - Unstake, + InFlightStaking, // TODO // Slash, } From 761876ad78b9acf0859c21f9161f6dceee6ebf47 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 10:21:04 +0200 Subject: [PATCH 13/47] Add commit / rollback vault api helpers --- packages/apis/src/vault_api.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/apis/src/vault_api.rs b/packages/apis/src/vault_api.rs index 541cfe85..92758e45 100644 --- a/packages/apis/src/vault_api.rs +++ b/packages/apis/src/vault_api.rs @@ -81,4 +81,24 @@ impl VaultApiHelper { }; Ok(wasm) } + + pub fn commit_tx(&self, tx_id: u64) -> Result { + let msg = VaultApiExecMsg::CommitTx { tx_id }; + let wasm = WasmMsg::Execute { + contract_addr: self.0.to_string(), + msg: to_binary(&msg)?, + funds: vec![], + }; + Ok(wasm) + } + + pub fn rollback_tx(&self, tx_id: u64) -> Result { + let msg = VaultApiExecMsg::RollbackTx { tx_id }; + let wasm = WasmMsg::Execute { + contract_addr: self.0.to_string(), + msg: to_binary(&msg)?, + funds: vec![], + }; + Ok(wasm) + } } From 2ead451220ea399f4f6abeff27c2ad62bd8241f8 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 10:40:07 +0200 Subject: [PATCH 14/47] Fix: unlock user on commit --- contracts/provider/vault/src/contract.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index ab022063..4489b6e0 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -520,6 +520,12 @@ impl VaultContract<'_> { (&ctx.info.sender, &tx.lienholder), &lien_lock, )?; + // Load user + let mut user_lock = self.users.load(ctx.deps.storage, &tx.user)?; + // Unlock it + user_lock.unlock_write()?; + // Save it + self.users.save(ctx.deps.storage, &tx.user, &user_lock)?; // Remove tx self.pending.txs.remove(ctx.deps.storage, tx_id)?; From 5c418bfa0bbcd86938d6f26915c3093112cf28af Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 10:51:29 +0200 Subject: [PATCH 15/47] Fix: proper lien user in commit / rollback --- contracts/provider/vault/src/contract.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 4489b6e0..cb681e7b 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -515,11 +515,8 @@ impl VaultContract<'_> { // Unlock it lien_lock.unlock_write()?; // Save it - self.liens.save( - ctx.deps.storage, - (&ctx.info.sender, &tx.lienholder), - &lien_lock, - )?; + self.liens + .save(ctx.deps.storage, (&tx.user, &tx.lienholder), &lien_lock)?; // Load user let mut user_lock = self.users.load(ctx.deps.storage, &tx.user)?; // Unlock it @@ -552,11 +549,8 @@ impl VaultContract<'_> { let lien = lien_lock.write()?; lien.amount -= tx.amount; // Save it unlocked - self.liens.save( - ctx.deps.storage, - (&ctx.info.sender, &tx.lienholder), - &lien_lock, - )?; + self.liens + .save(ctx.deps.storage, (&tx.user, &tx.lienholder), &lien_lock)?; // Load user let mut user_lock = self.users.load(ctx.deps.storage, &tx.user)?; From 72e2159d0febedbb984cbee02c42d1af5b43d67e Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 10:52:29 +0200 Subject: [PATCH 16/47] Transactional cross stake tests work (1st take) --- .../provider/vault/src/multitest/cross_staking.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/contracts/provider/vault/src/multitest/cross_staking.rs b/contracts/provider/vault/src/multitest/cross_staking.rs index 713785c5..0c514173 100644 --- a/contracts/provider/vault/src/multitest/cross_staking.rs +++ b/contracts/provider/vault/src/multitest/cross_staking.rs @@ -63,13 +63,18 @@ impl CrossStakingApi for CrossStaking<'_> { #[msg(exec)] fn receive_virtual_stake( &self, - _ctx: ExecCtx, + ctx: ExecCtx, _owner: String, _amount: Coin, - _tx: u64, + tx: u64, _msg: Binary, ) -> Result { - Ok(Response::new()) + let vault = ctx.info.sender; + let vault_api_helper = vault_api::VaultApiHelper(vault); + // TODO: Fail / rollback tx support + // FIXME: Shouldn't be sync + let msg = vault_api_helper.commit_tx(tx)?; + Ok(Response::new().add_message(msg)) } #[msg(query)] From 40f6005431aad79cd0e2a99e593edb047c461a4a Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 10:54:15 +0200 Subject: [PATCH 17/47] Fix syntax / details --- contracts/provider/vault/src/contract.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index cb681e7b..e3b9b491 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -342,9 +342,9 @@ impl VaultContract<'_> { /// Queries for all users ever performing action in the system, paginating over /// them. /// - /// `start_after` is the last accoutn inlcuded in previous page + /// `start_after` is the last account included in previous page /// - /// `with_collateral` flag filters out users with no collateral, defualted to false + /// `with_collateral` flag filters out users with no collateral, defaulted to false #[msg(query)] fn all_accounts( &self, From 6185e562b9d4c20e396f5f0ed5cad234056324cd Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 11:01:21 +0200 Subject: [PATCH 18/47] Fix clippy warning --- contracts/provider/vault/src/contract.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index e3b9b491..e847a0dc 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -450,10 +450,12 @@ impl VaultContract<'_> { let mut lien_lock = self .liens .may_load(ctx.deps.storage, (&ctx.info.sender, lienholder))? - .unwrap_or(Lockable::new(Lien { - amount: Uint128::zero(), - slashable, - })); + .unwrap_or_else(|| { + Lockable::new(Lien { + amount: Uint128::zero(), + slashable, + }) + }); let lien = lien_lock.write()?; lien.amount += amount; From effac898d29bd14df76235f296e1ceabf12cb135 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 12:02:20 +0200 Subject: [PATCH 19/47] Fix: receive virtual stake msg proper --- .../provider/external-staking/src/contract.rs | 4 ++-- contracts/provider/external-staking/src/msg.rs | 2 -- .../provider/external-staking/src/multitest.rs | 14 ++------------ 3 files changed, 4 insertions(+), 16 deletions(-) diff --git a/contracts/provider/external-staking/src/contract.rs b/contracts/provider/external-staking/src/contract.rs index 5146f472..a5b812fb 100644 --- a/contracts/provider/external-staking/src/contract.rs +++ b/contracts/provider/external-staking/src/contract.rs @@ -391,7 +391,7 @@ pub mod cross_staking { ctx: ExecCtx, owner: String, amount: Coin, - _tx_id: u64, + tx_id: u64, msg: Binary, ) -> Result { let config = self.config.load(ctx.deps.storage)?; @@ -436,7 +436,7 @@ pub mod cross_staking { .add_attribute("action", "receive_virtual_stake") .add_attribute("owner", owner) .add_attribute("amount", amount.amount.to_string()) - .add_attribute("tx_id", msg.tx.to_string()); + .add_attribute("tx_id", tx_id.to_string()); Ok(resp) } diff --git a/contracts/provider/external-staking/src/msg.rs b/contracts/provider/external-staking/src/msg.rs index 2bdb1737..08d30f9f 100644 --- a/contracts/provider/external-staking/src/msg.rs +++ b/contracts/provider/external-staking/src/msg.rs @@ -39,8 +39,6 @@ pub struct StakesResponse { /// Message to be send as `msg` field on `receive_virtual_staking` #[cw_serde] pub struct ReceiveVirtualStake { - /// Associated transaction id - pub tx: u64, pub validator: String, } diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index 3f110cd1..58bb1b20 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -120,7 +120,6 @@ fn staking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { - tx: 1, validator: validators[0].to_string(), }) .unwrap(), @@ -128,12 +127,13 @@ fn staking() { .call(users[0]) .unwrap(); + // Need to process / commit IBC tx here + vault .stake_remote( contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { - tx: 2, validator: validators[1].to_string(), }) .unwrap(), @@ -146,7 +146,6 @@ fn staking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { - tx: 3, validator: validators[0].to_string(), }) .unwrap(), @@ -159,7 +158,6 @@ fn staking() { contract.contract_addr.to_string(), coin(200, OSMO), to_binary(&ReceiveVirtualStake { - tx: 4, validator: validators[1].to_string(), }) .unwrap(), @@ -172,7 +170,6 @@ fn staking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { - tx: 5, validator: validators[0].to_string(), }) .unwrap(), @@ -299,7 +296,6 @@ fn unstaking() { contract.contract_addr.to_string(), coin(200, OSMO), to_binary(&ReceiveVirtualStake { - tx: 1, validator: validators[0].to_string(), }) .unwrap(), @@ -312,7 +308,6 @@ fn unstaking() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { - tx: 2, validator: validators[1].to_string(), }) .unwrap(), @@ -325,7 +320,6 @@ fn unstaking() { contract.contract_addr.to_string(), coin(300, OSMO), to_binary(&ReceiveVirtualStake { - tx: 3, validator: validators[0].to_string(), }) .unwrap(), @@ -585,7 +579,6 @@ fn distribution() { contract.contract_addr.to_string(), coin(200, OSMO), to_binary(&ReceiveVirtualStake { - tx: 1, validator: validators[0].to_string(), }) .unwrap(), @@ -598,7 +591,6 @@ fn distribution() { contract.contract_addr.to_string(), coin(100, OSMO), to_binary(&ReceiveVirtualStake { - tx: 2, validator: validators[1].to_string(), }) .unwrap(), @@ -611,7 +603,6 @@ fn distribution() { contract.contract_addr.to_string(), coin(300, OSMO), to_binary(&ReceiveVirtualStake { - tx: 3, validator: validators[0].to_string(), }) .unwrap(), @@ -829,7 +820,6 @@ fn distribution() { contract.contract_addr.to_string(), coin(300, OSMO), to_binary(&ReceiveVirtualStake { - tx: 2, validator: validators[1].to_string(), }) .unwrap(), From 384c12bbd4426f39f13f8cf8b70e0cfc9d22b5ff Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 13:16:30 +0200 Subject: [PATCH 20/47] Fix staking test --- .../external-staking/src/multitest.rs | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index 58bb1b20..31264aed 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -3,6 +3,7 @@ use mesh_native_staking::contract::multitest_utils::CodeId as NativeStakingCodeI use mesh_native_staking::contract::InstantiateMsg as NativeStakingInstantiateMsg; use mesh_native_staking_proxy::contract::multitest_utils::CodeId as NativeStakingProxyCodeId; use mesh_vault::contract::multitest_utils::{CodeId as VaultCodeId, VaultContractProxy}; +use mesh_vault::contract::test_utils::VaultApi; use mesh_vault::msg::StakingInitInfo; use cw_multi_test::App as MtApp; @@ -127,7 +128,14 @@ fn staking() { .call(users[0]) .unwrap(); - // Need to process / commit IBC tx here + // Hardcoded commit tx call + // TODO: get last tx helper + let mut last_tx = 1; + vault + .vault_api_proxy() + .commit_tx(last_tx) + .call(contract.contract_addr.as_str()) + .unwrap(); vault .stake_remote( @@ -141,6 +149,13 @@ fn staking() { .call(users[0]) .unwrap(); + last_tx += 1; + vault + .vault_api_proxy() + .commit_tx(last_tx) + .call(contract.contract_addr.as_str()) + .unwrap(); + vault .stake_remote( contract.contract_addr.to_string(), @@ -153,6 +168,13 @@ fn staking() { .call(users[0]) .unwrap(); + last_tx += 1; + vault + .vault_api_proxy() + .commit_tx(last_tx) + .call(contract.contract_addr.as_str()) + .unwrap(); + vault .stake_remote( contract.contract_addr.to_string(), @@ -165,6 +187,13 @@ fn staking() { .call(users[1]) .unwrap(); + last_tx += 1; + vault + .vault_api_proxy() + .commit_tx(last_tx) + .call(contract.contract_addr.as_str()) + .unwrap(); + vault .stake_remote( contract.contract_addr.to_string(), @@ -177,6 +206,13 @@ fn staking() { .call(users[1]) .unwrap(); + last_tx += 1; + vault + .vault_api_proxy() + .commit_tx(last_tx) + .call(contract.contract_addr.as_str()) + .unwrap(); + // All tokens should be only on the vault contract assert_eq!(app.app().wrap().query_all_balances(users[0]).unwrap(), []); assert_eq!(app.app().wrap().query_all_balances(users[1]).unwrap(), []); From 7fcda86f45bbd6802bfd4144c9e2a5f7395bfe6a Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 13:28:30 +0200 Subject: [PATCH 21/47] Add all pending txs query --- contracts/provider/vault/src/contract.rs | 33 +++++++++++++++++++++--- contracts/provider/vault/src/msg.rs | 7 +++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index e847a0dc..17d29dd7 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -16,10 +16,7 @@ use sylvia::types::{ExecCtx, InstantiateCtx, QueryCtx, ReplyCtx}; use sylvia::{contract, schemars}; use crate::error::ContractError; -use crate::msg::{ - AccountClaimsResponse, AccountResponse, AllAccountsResponse, AllAccountsResponseItem, - ConfigResponse, LienInfo, StakingInitInfo, -}; +use crate::msg::{AccountClaimsResponse, AccountResponse, AllAccountsResponse, AllAccountsResponseItem, AllTxsResponse, AllTxsResponseItem, ConfigResponse, LienInfo, StakingInitInfo}; use crate::state::{Config, Lien, LocalStaking, UserInfo}; use crate::txs::{Tx, TxType, Txs}; @@ -393,6 +390,34 @@ impl VaultContract<'_> { Ok(resp) } + /// Queries for all pending txs. + /// `start_after` is the last tx id included in previous page + #[msg(query)] + fn all_pending_txs( + &self, + ctx: QueryCtx, + start_after: Option, + limit: Option, + ) -> Result { + let limit = clamp_page_limit(limit); + let bound = start_after.and_then(Bounder::exclusive_bound); + + let txs = self + .pending + .txs + .range(ctx.deps.storage, bound, None, Order::Ascending) + .map(|item| { + let (_id, tx) = item?; + Ok::(tx) + }) + .take(limit) + .collect::>()?; + + let resp = AllTxsResponse { txs }; + + Ok(resp) + } + #[msg(reply)] fn reply(&self, ctx: ReplyCtx, reply: Reply) -> Result { match reply.id { diff --git a/contracts/provider/vault/src/msg.rs b/contracts/provider/vault/src/msg.rs index e5c97ac8..6f8804cd 100644 --- a/contracts/provider/vault/src/msg.rs +++ b/contracts/provider/vault/src/msg.rs @@ -52,3 +52,10 @@ pub struct ConfigResponse { pub denom: String, pub local_staking: String, } + +pub type AllTxsResponseItem = crate::txs::Tx; + +#[cw_serde] +pub struct AllTxsResponse { + pub txs: Vec, +} From 2b43371e1e08e5c8a4359346158f0cb9fb7e310f Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 13:33:34 +0200 Subject: [PATCH 22/47] Use pending txs query to get tx id --- .../external-staking/src/multitest.rs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index 31264aed..1ea0a02d 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -128,9 +128,7 @@ fn staking() { .call(users[0]) .unwrap(); - // Hardcoded commit tx call - // TODO: get last tx helper - let mut last_tx = 1; + let last_tx = get_last_pending_tx_id(&vault).unwrap(); vault .vault_api_proxy() .commit_tx(last_tx) @@ -149,10 +147,9 @@ fn staking() { .call(users[0]) .unwrap(); - last_tx += 1; vault .vault_api_proxy() - .commit_tx(last_tx) + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) .call(contract.contract_addr.as_str()) .unwrap(); @@ -168,10 +165,9 @@ fn staking() { .call(users[0]) .unwrap(); - last_tx += 1; vault .vault_api_proxy() - .commit_tx(last_tx) + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) .call(contract.contract_addr.as_str()) .unwrap(); @@ -187,10 +183,9 @@ fn staking() { .call(users[1]) .unwrap(); - last_tx += 1; vault .vault_api_proxy() - .commit_tx(last_tx) + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) .call(contract.contract_addr.as_str()) .unwrap(); @@ -205,11 +200,9 @@ fn staking() { ) .call(users[1]) .unwrap(); - - last_tx += 1; vault .vault_api_proxy() - .commit_tx(last_tx) + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) .call(contract.contract_addr.as_str()) .unwrap(); @@ -288,6 +281,12 @@ fn staking() { ); } +#[track_caller] +fn get_last_pending_tx_id(vault: &VaultContractProxy) -> Option { + let txs = vault.all_pending_txs(None, None).unwrap().txs; + txs.last().map(|tx| tx.id) +} + #[test] fn unstaking() { let users = ["user1", "user2"]; From a7c28ad942a8a2c361780fcb1a657694dfb5877d Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 13:47:29 +0200 Subject: [PATCH 23/47] Report pending txs in descending order (newest first) --- contracts/provider/external-staking/src/multitest.rs | 2 +- contracts/provider/vault/src/contract.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index 1ea0a02d..85c8c23e 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -284,7 +284,7 @@ fn staking() { #[track_caller] fn get_last_pending_tx_id(vault: &VaultContractProxy) -> Option { let txs = vault.all_pending_txs(None, None).unwrap().txs; - txs.last().map(|tx| tx.id) + txs.first().map(|tx| tx.id) } #[test] diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 17d29dd7..58d05cdf 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -391,6 +391,7 @@ impl VaultContract<'_> { } /// Queries for all pending txs. + /// Reports txs in descending order (newest first). /// `start_after` is the last tx id included in previous page #[msg(query)] fn all_pending_txs( @@ -405,7 +406,7 @@ impl VaultContract<'_> { let txs = self .pending .txs - .range(ctx.deps.storage, bound, None, Order::Ascending) + .range(ctx.deps.storage, bound, None, Order::Descending) .map(|item| { let (_id, tx) = item?; Ok::(tx) From 4ef5fdbea633aaf1ee6ee3cc68cba127933bee5f Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 13:53:32 +0200 Subject: [PATCH 24/47] Fix: range limits --- contracts/provider/vault/src/contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 58d05cdf..ca4e3668 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -406,7 +406,7 @@ impl VaultContract<'_> { let txs = self .pending .txs - .range(ctx.deps.storage, bound, None, Order::Descending) + .range(ctx.deps.storage, None, bound, Order::Descending) .map(|item| { let (_id, tx) = item?; Ok::(tx) From 6f7eb40b3b361eb726f5465d2d3bda27e2e72570 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 13:57:33 +0200 Subject: [PATCH 25/47] Fix unstaking test --- .../provider/external-staking/src/multitest.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index 85c8c23e..d8fe6562 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -129,6 +129,7 @@ fn staking() { .unwrap(); let last_tx = get_last_pending_tx_id(&vault).unwrap(); + // Hardcoded commit_tx call (lack of IBC support yet) vault .vault_api_proxy() .commit_tx(last_tx) @@ -337,6 +338,13 @@ fn unstaking() { ) .call(users[0]) .unwrap(); + let last_tx = get_last_pending_tx_id(&vault).unwrap(); + // Hardcoded commit_tx call (lack of IBC support yet) + vault + .vault_api_proxy() + .commit_tx(last_tx) + .call(contract.contract_addr.as_str()) + .unwrap(); vault .stake_remote( @@ -349,6 +357,11 @@ fn unstaking() { ) .call(users[0]) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(contract.contract_addr.as_str()) + .unwrap(); vault .stake_remote( @@ -361,6 +374,11 @@ fn unstaking() { ) .call(users[1]) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(contract.contract_addr.as_str()) + .unwrap(); // Properly unstake some tokens // users[0] unstakes 50 from validators[0] - 150 left staken in 2 batches From e0bf55e0f2361ec51884630f55169074e7e7ce4f Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 13:59:09 +0200 Subject: [PATCH 26/47] Fix distribution test --- .../provider/external-staking/src/multitest.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index d8fe6562..0d57f8d2 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -638,6 +638,12 @@ fn distribution() { ) .call(users[0]) .unwrap(); + // Hardcoded commit_tx call (lack of IBC support yet) + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(contract.contract_addr.as_str()) + .unwrap(); vault .stake_remote( @@ -650,6 +656,11 @@ fn distribution() { ) .call(users[0]) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(contract.contract_addr.as_str()) + .unwrap(); vault .stake_remote( @@ -662,6 +673,11 @@ fn distribution() { ) .call(users[1]) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(contract.contract_addr.as_str()) + .unwrap(); // Start with equal distribution: // 20 tokens for users[0] From 8150e892cda45feb646ecf93bc31e28b843a5f9a Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 14:03:40 +0200 Subject: [PATCH 27/47] Fix clippy warnings --- contracts/provider/vault/src/contract.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index ca4e3668..a9970bf8 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -16,7 +16,10 @@ use sylvia::types::{ExecCtx, InstantiateCtx, QueryCtx, ReplyCtx}; use sylvia::{contract, schemars}; use crate::error::ContractError; -use crate::msg::{AccountClaimsResponse, AccountResponse, AllAccountsResponse, AllAccountsResponseItem, AllTxsResponse, AllTxsResponseItem, ConfigResponse, LienInfo, StakingInitInfo}; +use crate::msg::{ + AccountClaimsResponse, AccountResponse, AllAccountsResponse, AllAccountsResponseItem, + AllTxsResponse, AllTxsResponseItem, ConfigResponse, LienInfo, StakingInitInfo, +}; use crate::state::{Config, Lien, LocalStaking, UserInfo}; use crate::txs::{Tx, TxType, Txs}; From c2b18f8d8b7bfa255e7c971654e7ed309409709d Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 14:53:36 +0200 Subject: [PATCH 28/47] Manual commit_tx call for staking tests --- contracts/provider/vault/src/multitest.rs | 46 +++++++++++++++++++ .../vault/src/multitest/cross_staking.rs | 11 ++--- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index 44b54df1..98d9a4d0 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -1,6 +1,8 @@ mod cross_staking; mod local_staking; +use crate::contract::multitest_utils::VaultContractProxy; +use crate::contract::test_utils::VaultApi; use crate::error::ContractError; use crate::msg::{AllAccountsResponseItem, LienInfo, StakingInitInfo}; use crate::{contract, msg::AccountResponse}; @@ -441,6 +443,12 @@ fn stake_local() { .unwrap_err(); } +#[track_caller] +fn get_last_pending_tx_id(vault: &VaultContractProxy) -> Option { + let txs = vault.all_pending_txs(None, None).unwrap().txs; + txs.first().map(|tx| tx.id) +} + #[test] fn stake_cross() { let owner = "owner"; @@ -522,6 +530,14 @@ fn stake_cross() { .call(user) .unwrap(); + let last_tx = get_last_pending_tx_id(&vault).unwrap(); + // Hardcoded commit_tx call (lack of IBC support yet) + vault + .vault_api_proxy() + .commit_tx(last_tx) + .call(cross_staking.contract_addr.as_str()) + .unwrap(); + let acc = vault.account(user.to_owned()).unwrap(); assert_eq!( acc, @@ -562,6 +578,11 @@ fn stake_cross() { ) .call(user) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(cross_staking.contract_addr.as_str()) + .unwrap(); let acc = vault.account(user.to_owned()).unwrap(); assert_eq!( @@ -783,6 +804,11 @@ fn multiple_stakes() { ) .call(user) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(cross_staking1.contract_addr.as_str()) + .unwrap(); vault .stake_remote( @@ -792,6 +818,11 @@ fn multiple_stakes() { ) .call(user) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(cross_staking2.contract_addr.as_str()) + .unwrap(); assert_eq!( vault.account(user.to_owned()).unwrap(), @@ -834,6 +865,11 @@ fn multiple_stakes() { ) .call(user) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(cross_staking1.contract_addr.as_str()) + .unwrap(); vault .stake_remote( @@ -843,6 +879,11 @@ fn multiple_stakes() { ) .call(user) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(cross_staking2.contract_addr.as_str()) + .unwrap(); assert_eq!( vault.account(user.to_owned()).unwrap(), @@ -883,6 +924,11 @@ fn multiple_stakes() { ) .call(user) .unwrap(); + vault + .vault_api_proxy() + .commit_tx(get_last_pending_tx_id(&vault).unwrap()) + .call(cross_staking1.contract_addr.as_str()) + .unwrap(); let err = vault .stake_remote( diff --git a/contracts/provider/vault/src/multitest/cross_staking.rs b/contracts/provider/vault/src/multitest/cross_staking.rs index 0c514173..713785c5 100644 --- a/contracts/provider/vault/src/multitest/cross_staking.rs +++ b/contracts/provider/vault/src/multitest/cross_staking.rs @@ -63,18 +63,13 @@ impl CrossStakingApi for CrossStaking<'_> { #[msg(exec)] fn receive_virtual_stake( &self, - ctx: ExecCtx, + _ctx: ExecCtx, _owner: String, _amount: Coin, - tx: u64, + _tx: u64, _msg: Binary, ) -> Result { - let vault = ctx.info.sender; - let vault_api_helper = vault_api::VaultApiHelper(vault); - // TODO: Fail / rollback tx support - // FIXME: Shouldn't be sync - let msg = vault_api_helper.commit_tx(tx)?; - Ok(Response::new().add_message(msg)) + Ok(Response::new()) } #[msg(query)] From a6be2e9936c8b3952b30dd86f36412f232c275d6 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 16:14:08 +0200 Subject: [PATCH 29/47] Add staking txs / locks test --- contracts/provider/vault/src/multitest.rs | 209 ++++++++++++++++++++++ 1 file changed, 209 insertions(+) diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index 98d9a4d0..1d2f2833 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -7,7 +7,9 @@ use crate::error::ContractError; use crate::msg::{AllAccountsResponseItem, LienInfo, StakingInitInfo}; use crate::{contract, msg::AccountResponse}; use cosmwasm_std::{coin, coins, to_binary, Addr, Binary, Decimal, Empty, Uint128}; +use cosmwasm_std::StdError::GenericErr; use cw_multi_test::App as MtApp; +use mesh_sync::LockError; use sylvia::multitest::App; const OSMO: &str = "OSMO"; @@ -732,6 +734,213 @@ fn stake_cross() { .unwrap_err(); } +#[test] +fn stake_cross_txs() { + let owner = "owner"; + let user = "user1"; + let user2 = "user2"; + + let app = MtApp::new(|router, _api, storage| { + router + .bank + .init_balance(storage, &Addr::unchecked(user), coins(300, OSMO)) + .unwrap(); + router + .bank + .init_balance(storage, &Addr::unchecked(user2), coins(500, OSMO)) + .unwrap(); + }); + let app = App::new(app); + + // Contracts setup + + let local_staking_code = local_staking::multitest_utils::CodeId::store_code(&app); + let cross_staking_code = cross_staking::multitest_utils::CodeId::store_code(&app); + let vault_code = contract::multitest_utils::CodeId::store_code(&app); + + let cross_staking = cross_staking_code + .instantiate(Decimal::percent(10)) + .call(owner) + .unwrap(); + + let staking_init_info = StakingInitInfo { + admin: None, + code_id: local_staking_code.code_id(), + msg: to_binary(&Empty {}).unwrap(), + label: None, + }; + + let vault = vault_code + .instantiate(OSMO.to_owned(), staking_init_info) + .with_label("Vault") + .call(owner) + .unwrap(); + + // Bond some tokens + + vault + .bond() + .with_funds(&coins(300, OSMO)) + .call(user) + .unwrap(); + + assert_eq!( + vault.account(user.to_owned()).unwrap(), + AccountResponse { + denom: OSMO.to_owned(), + bonded: Uint128::new(300), + free: Uint128::new(300), + } + ); + let claims = vault.account_claims(user.to_owned(), None, None).unwrap(); + assert_eq!(claims.claims, []); + + vault + .bond() + .with_funds(&coins(500, OSMO)) + .call(user2) + .unwrap(); + assert_eq!( + vault.account(user2.to_owned()).unwrap(), + AccountResponse { + denom: OSMO.to_owned(), + bonded: Uint128::new(500), + free: Uint128::new(500), + } + ); + assert_eq!( + app.app() + .wrap() + .query_balance(&vault.contract_addr, OSMO) + .unwrap(), + coin(800, OSMO) + ); + assert_eq!( + app.app() + .wrap() + .query_balance(&cross_staking.contract_addr, OSMO) + .unwrap(), + coin(0, OSMO) + ); + + // No pending txs + assert_eq!(vault.all_pending_txs(None, None).unwrap().txs, vec![]); + + // Staking remotely + + vault + .stake_remote( + cross_staking.contract_addr.to_string(), + coin(100, OSMO), + Binary::default(), + ) + .call(user) + .unwrap(); + + // One pending tx + assert_eq!(vault.all_pending_txs(None, None).unwrap().txs.len(), 1); + + // Same user cannot stake while pending tx + assert_eq!( + vault + .stake_remote( + cross_staking.contract_addr.to_string(), + coin(100, OSMO), + Binary::default(), + ) + .call(user) + .unwrap_err(), + ContractError::Lock(LockError::WriteLocked) + ); + // Store for later + let first_tx = get_last_pending_tx_id(&vault).unwrap(); + + // But other user can + vault + .stake_remote( + cross_staking.contract_addr.to_string(), + coin(100, OSMO), + Binary::default(), + ) + .call(user2) + .unwrap(); + + // Two pending txs + assert_eq!(vault.all_pending_txs(None, None).unwrap().txs.len(), 2); + + // Last tx commit_tx call + let last_tx = get_last_pending_tx_id(&vault).unwrap(); + vault + .vault_api_proxy() + .commit_tx(last_tx) + .call(cross_staking.contract_addr.as_str()) + .unwrap(); + + // First tx is still pending + assert_eq!(vault.all_pending_txs(None, None).unwrap().txs[0].id, first_tx); + + // Cannot query account while pending + assert!(matches!(vault.account(user.to_owned()).unwrap_err(), + ContractError::Std(GenericErr {..}))); // write locked + // Cannot query claims while pending + assert!(matches!(vault.account_claims(user.to_owned(), None, None).unwrap_err(), + ContractError::Std(GenericErr {..}))); // write locked + + // Can query the other account + let acc = vault.account(user2.to_owned()).unwrap(); + assert_eq!( + acc, + AccountResponse { + denom: OSMO.to_owned(), + bonded: Uint128::new(500), + free: Uint128::new(400), + } + ); + // Can query the other account claims + let claims = vault.account_claims(user2.to_owned(), None, None).unwrap(); + assert_eq!( + claims.claims, + [LienInfo { + lienholder: cross_staking.contract_addr.to_string(), + amount: Uint128::new(100) + }] + ); + + // Commit first tx + vault + .vault_api_proxy() + .commit_tx(first_tx) + .call(cross_staking.contract_addr.as_str()) + .unwrap(); + + // Can query account now + let acc = vault.account(user.to_owned()).unwrap(); + assert_eq!( + acc, + AccountResponse { + denom: OSMO.to_owned(), + bonded: Uint128::new(300), + free: Uint128::new(200), + } + ); + // Can query claims + let claims = vault.account_claims(user.to_owned(), None, None).unwrap(); + assert_eq!( + claims.claims, + [LienInfo { + lienholder: cross_staking.contract_addr.to_string(), + amount: Uint128::new(100) + }] + ); + assert_eq!( + app.app() + .wrap() + .query_balance(&vault.contract_addr, OSMO) + .unwrap(), + coin(800, OSMO) + ); +} + #[test] fn multiple_stakes() { let owner = "owner"; From 223551ec6336de597936e622c01a7e4a445f0848 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 16:17:39 +0200 Subject: [PATCH 30/47] Add query vault balance check --- contracts/provider/vault/src/multitest.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index 1d2f2833..09052dca 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -885,6 +885,14 @@ fn stake_cross_txs() { // Cannot query claims while pending assert!(matches!(vault.account_claims(user.to_owned(), None, None).unwrap_err(), ContractError::Std(GenericErr {..}))); // write locked + // Can query vault's balance while pending + assert_eq!( + app.app() + .wrap() + .query_balance(&vault.contract_addr, OSMO) + .unwrap(), + coin(800, OSMO) + ); // Can query the other account let acc = vault.account(user2.to_owned()).unwrap(); From ab378988ac59addf4cfe6b5733090697f2db1ee8 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 18:39:08 +0200 Subject: [PATCH 31/47] Fix: error filtering --- contracts/provider/vault/src/contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index a9970bf8..98d2a846 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -370,7 +370,7 @@ impl VaultContract<'_> { // FIXME: This skips write-locked accounts match account_lock.read() { Ok(account) => account.collateral.is_zero(), - Err(_) => false, + Err(_) => true, } }) .unwrap_or(false)) From 17fcf86c84495a3362801ecc6d7febdd7d3c7a2d Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 18:40:00 +0200 Subject: [PATCH 32/47] Add all accounts checks with locked items Locked range fails with ParseError --- contracts/provider/vault/src/multitest.rs | 42 ++++++++++++++++++----- 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index 09052dca..474234e2 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -6,8 +6,8 @@ use crate::contract::test_utils::VaultApi; use crate::error::ContractError; use crate::msg::{AllAccountsResponseItem, LienInfo, StakingInitInfo}; use crate::{contract, msg::AccountResponse}; -use cosmwasm_std::{coin, coins, to_binary, Addr, Binary, Decimal, Empty, Uint128}; use cosmwasm_std::StdError::GenericErr; +use cosmwasm_std::{coin, coins, to_binary, Addr, Binary, Decimal, Empty, Uint128}; use cw_multi_test::App as MtApp; use mesh_sync::LockError; use sylvia::multitest::App; @@ -825,6 +825,9 @@ fn stake_cross_txs() { // No pending txs assert_eq!(vault.all_pending_txs(None, None).unwrap().txs, vec![]); + // Can query all accounts + let accounts = vault.all_accounts(false, None, None).unwrap(); + assert_eq!(accounts.accounts.len(), 2); // Staking remotely @@ -877,15 +880,24 @@ fn stake_cross_txs() { .unwrap(); // First tx is still pending - assert_eq!(vault.all_pending_txs(None, None).unwrap().txs[0].id, first_tx); + assert_eq!( + vault.all_pending_txs(None, None).unwrap().txs[0].id, + first_tx + ); // Cannot query account while pending - assert!(matches!(vault.account(user.to_owned()).unwrap_err(), - ContractError::Std(GenericErr {..}))); // write locked - // Cannot query claims while pending - assert!(matches!(vault.account_claims(user.to_owned(), None, None).unwrap_err(), - ContractError::Std(GenericErr {..}))); // write locked - // Can query vault's balance while pending + assert!(matches!( + vault.account(user.to_owned()).unwrap_err(), + ContractError::Std(GenericErr { .. }) + )); // write locked + // Cannot query claims while pending + assert!(matches!( + vault + .account_claims(user.to_owned(), None, None) + .unwrap_err(), + ContractError::Std(GenericErr { .. }) + )); // write locked + // Can query vault's balance while pending assert_eq!( app.app() .wrap() @@ -893,6 +905,20 @@ fn stake_cross_txs() { .unwrap(), coin(800, OSMO) ); + // Can query all accounts, but locked ones are not returned + // FIXME: range() fails with ParseError! + // let accounts = vault.all_accounts(false, None, None).unwrap(); + // assert_eq!( + // accounts.accounts, + // [AllAccountsResponseItem { + // account: user2.to_string(), + // denom: OSMO.to_owned(), + // bonded: Uint128::new(500), + // free: Uint128::new(400), + // }] + // ); + let err = vault.all_accounts(false, None, None).unwrap_err(); + println!("err: {:#?}", err); // Can query the other account let acc = vault.account(user2.to_owned()).unwrap(); From a2a21b364e79483fa745c7204218fb8aba636a0a Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 20:45:22 +0200 Subject: [PATCH 33/47] Fix: user unlocking in rollback tx --- contracts/provider/vault/src/contract.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 98d2a846..d64855cc 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -586,7 +586,7 @@ impl VaultContract<'_> { // Load user let mut user_lock = self.users.load(ctx.deps.storage, &tx.user)?; // Rollback user's max_lien (need to unlock it first) - lien_lock.unlock_write()?; + user_lock.unlock_write()?; let mut user = user_lock.write()?; // Max lien has to be recalculated from scratch; the just rolled back lien From bece088d07c5c8a035d230a1a338c833733002d5 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Fri, 16 Jun 2023 20:45:58 +0200 Subject: [PATCH 34/47] Add rollback tx test --- contracts/provider/vault/src/multitest.rs | 108 ++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index 474234e2..c865f3f3 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -975,6 +975,114 @@ fn stake_cross_txs() { ); } +#[test] +fn stake_cross_rollback_tx() { + let owner = "owner"; + let user = "user1"; + + let app = MtApp::new(|router, _api, storage| { + router + .bank + .init_balance(storage, &Addr::unchecked(user), coins(300, OSMO)) + .unwrap(); + }); + let app = App::new(app); + + // Contracts setup + + let local_staking_code = local_staking::multitest_utils::CodeId::store_code(&app); + let cross_staking_code = cross_staking::multitest_utils::CodeId::store_code(&app); + let vault_code = contract::multitest_utils::CodeId::store_code(&app); + + let cross_staking = cross_staking_code + .instantiate(Decimal::percent(10)) + .call(owner) + .unwrap(); + + let staking_init_info = StakingInitInfo { + admin: None, + code_id: local_staking_code.code_id(), + msg: to_binary(&Empty {}).unwrap(), + label: None, + }; + + let vault = vault_code + .instantiate(OSMO.to_owned(), staking_init_info) + .with_label("Vault") + .call(owner) + .unwrap(); + + // Bond some tokens + + vault + .bond() + .with_funds(&coins(300, OSMO)) + .call(user) + .unwrap(); + + assert_eq!( + vault.account(user.to_owned()).unwrap(), + AccountResponse { + denom: OSMO.to_owned(), + bonded: Uint128::new(300), + free: Uint128::new(300), + } + ); + + // Staking remotely + + vault + .stake_remote( + cross_staking.contract_addr.to_string(), + coin(100, OSMO), + Binary::default(), + ) + .call(user) + .unwrap(); + + // One pending tx + assert_eq!(vault.all_pending_txs(None, None).unwrap().txs.len(), 1); + + // Rollback tx + let last_tx = get_last_pending_tx_id(&vault).unwrap(); + vault + .vault_api_proxy() + .rollback_tx(last_tx) + .call(cross_staking.contract_addr.as_str()) + .unwrap(); + + // No pending txs + assert!(vault.all_pending_txs(None, None).unwrap().txs.is_empty()); + + // Funds are restored + let acc = vault.account(user.to_owned()).unwrap(); + assert_eq!( + acc, + AccountResponse { + denom: OSMO.to_owned(), + bonded: Uint128::new(300), + free: Uint128::new(300), + } + ); + // No non-empty claims + let claims = vault.account_claims(user.to_owned(), None, None).unwrap(); + assert_eq!( + claims.claims, + [LienInfo { + lienholder: cross_staking.contract_addr.to_string(), + amount: Uint128::new(0) + }] + ); + // Vault has the funds + assert_eq!( + app.app() + .wrap() + .query_balance(&vault.contract_addr, OSMO) + .unwrap(), + coin(300, OSMO) + ); +} + #[test] fn multiple_stakes() { let owner = "owner"; From 8860a9e88c1ee22b31ef0eebfaf158ca49d61f8f Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Sat, 17 Jun 2023 17:00:27 +0200 Subject: [PATCH 35/47] Fix: skip write locked accounts --- contracts/provider/vault/src/contract.rs | 24 +++++++++++------------ contracts/provider/vault/src/multitest.rs | 23 ++++++++++------------ 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index d64855cc..9c9b0b7a 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -359,21 +359,19 @@ impl VaultContract<'_> { let denom = self.config.load(ctx.deps.storage)?.denom; - let accounts = self + let accounts: Vec<_> = self .users .range(ctx.deps.storage, bound, None, Order::Ascending) .filter(|account| { - !(with_collateral - && account - .as_ref() - .map(|(_, account_lock)| { - // FIXME: This skips write-locked accounts - match account_lock.read() { - Ok(account) => account.collateral.is_zero(), - Err(_) => true, - } - }) - .unwrap_or(false)) + account + .as_ref() + .map(|(_, account_lock)| { + match account_lock.read() { + Ok(account) => !with_collateral || !account.collateral.is_zero(), // Skip zero collateral + Err(_) => false, // Skip write-locked accounts + } + }) + .unwrap_or(false) }) .map(|account| { let (addr, account_lock) = account?; @@ -386,7 +384,7 @@ impl VaultContract<'_> { }) }) .take(limit) - .collect::>()?; + .collect::>()?; let resp = AllAccountsResponse { accounts }; diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index c865f3f3..d248b239 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -906,19 +906,16 @@ fn stake_cross_txs() { coin(800, OSMO) ); // Can query all accounts, but locked ones are not returned - // FIXME: range() fails with ParseError! - // let accounts = vault.all_accounts(false, None, None).unwrap(); - // assert_eq!( - // accounts.accounts, - // [AllAccountsResponseItem { - // account: user2.to_string(), - // denom: OSMO.to_owned(), - // bonded: Uint128::new(500), - // free: Uint128::new(400), - // }] - // ); - let err = vault.all_accounts(false, None, None).unwrap_err(); - println!("err: {:#?}", err); + let accounts = vault.all_accounts(false, None, None).unwrap(); + assert_eq!( + accounts.accounts, + [AllAccountsResponseItem { + account: user2.to_string(), + denom: OSMO.to_owned(), + bonded: Uint128::new(500), + free: Uint128::new(400), + }] + ); // Can query the other account let acc = vault.account(user2.to_owned()).unwrap(); From fec475aded23bb674c312ed6adbb4833cf7042bc Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Sat, 17 Jun 2023 17:03:15 +0200 Subject: [PATCH 36/47] Idiomatic filter / maps --- contracts/provider/vault/src/contract.rs | 28 +++++++++++++----------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 9c9b0b7a..d576247a 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -366,22 +366,24 @@ impl VaultContract<'_> { account .as_ref() .map(|(_, account_lock)| { - match account_lock.read() { - Ok(account) => !with_collateral || !account.collateral.is_zero(), // Skip zero collateral - Err(_) => false, // Skip write-locked accounts - } + account_lock + .read() + .map(|account| !with_collateral || !account.collateral.is_zero()) // Skip zero collateral + .unwrap_or(false) // Skip write-locked accounts }) - .unwrap_or(false) + .unwrap_or(false) // Skip other errors }) .map(|account| { - let (addr, account_lock) = account?; - let account = account_lock.read()?; - Ok::(AllAccountsResponseItem { - account: addr.into(), - denom: denom.clone(), - bonded: account.collateral, - free: account.free_collateral(), - }) + account.map(|(addr, account_lock)| { + account_lock.read().map(|account| { + Ok(AllAccountsResponseItem { + account: addr.into(), + denom: denom.clone(), + bonded: account.collateral, + free: account.free_collateral(), + }) + })? + })? }) .take(limit) .collect::>()?; From a58f7c1ab501a28cd3087a7804b0daab9dda9517 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Mon, 19 Jun 2023 10:46:01 +0200 Subject: [PATCH 37/47] Add pending tx query --- contracts/provider/vault/src/contract.rs | 9 ++++++++- contracts/provider/vault/src/msg.rs | 3 ++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index d576247a..1f98f1e5 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -18,7 +18,7 @@ use sylvia::{contract, schemars}; use crate::error::ContractError; use crate::msg::{ AccountClaimsResponse, AccountResponse, AllAccountsResponse, AllAccountsResponseItem, - AllTxsResponse, AllTxsResponseItem, ConfigResponse, LienInfo, StakingInitInfo, + AllTxsResponse, AllTxsResponseItem, ConfigResponse, LienInfo, StakingInitInfo, TxResponse, }; use crate::state::{Config, Lien, LocalStaking, UserInfo}; use crate::txs::{Tx, TxType, Txs}; @@ -393,6 +393,13 @@ impl VaultContract<'_> { Ok(resp) } + /// Queries a pending tx. + #[msg(query)] + fn pending_tx(&self, ctx: QueryCtx, tx_id: u64) -> Result { + let resp = self.pending.txs.load(ctx.deps.storage, tx_id)?; + Ok(resp) + } + /// Queries for all pending txs. /// Reports txs in descending order (newest first). /// `start_after` is the last tx id included in previous page diff --git a/contracts/provider/vault/src/msg.rs b/contracts/provider/vault/src/msg.rs index 6f8804cd..02c225b8 100644 --- a/contracts/provider/vault/src/msg.rs +++ b/contracts/provider/vault/src/msg.rs @@ -53,7 +53,8 @@ pub struct ConfigResponse { pub local_staking: String, } -pub type AllTxsResponseItem = crate::txs::Tx; +pub type TxResponse = crate::txs::Tx; +pub type AllTxsResponseItem = TxResponse; #[cw_serde] pub struct AllTxsResponse { From 856b74f65b24efeae01a117d7c830e3499f9227d Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:04:21 +0200 Subject: [PATCH 38/47] Move Tx to sync package Use enum instead of struct for generality / extensibility --- packages/sync/src/lib.rs | 2 ++ packages/sync/src/txs.rs | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 packages/sync/src/txs.rs diff --git a/packages/sync/src/lib.rs b/packages/sync/src/lib.rs index b29414ae..dff475a8 100644 --- a/packages/sync/src/lib.rs +++ b/packages/sync/src/lib.rs @@ -1,5 +1,7 @@ mod locks; mod range; +mod txs; pub use locks::{LockError, LockState, Lockable}; pub use range::{max_range, min_range, spread, RangeError, ValueRange}; +pub use txs::Tx; diff --git a/packages/sync/src/txs.rs b/packages/sync/src/txs.rs new file mode 100644 index 00000000..200299fa --- /dev/null +++ b/packages/sync/src/txs.rs @@ -0,0 +1,38 @@ +use cosmwasm_schema::cw_serde; +use cosmwasm_std::{Addr, Decimal, Uint128}; +use std::fmt::Formatter; + +#[cw_serde] +pub enum Tx { + InFlightStaking { + /// Transaction id + id: u64, + /// Associated amount + amount: Uint128, + /// Slashable portion of lien + slashable: Decimal, + /// Associated user + user: Addr, + /// Remote staking contract + lienholder: Addr, + }, + // TODO: + // InFlightSlashing +} + +// Implement display for Tx +impl std::fmt::Display for Tx { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self { + Tx::InFlightStaking { + id, + amount, + slashable, + user, + lienholder, + } => { + write!(f, "InFlightStaking {{ id: {}, amount: {}, slashable: {}, user: {}, lienholder: {} }}", id, amount, slashable, user, lienholder) + } + } + } +} From f92fb8379ed73fde67dd7bc9aba7fcbba057c62a Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:18:34 +0200 Subject: [PATCH 39/47] Adapt vault to new Tx format --- contracts/provider/vault/src/contract.rs | 77 ++++++++++++++++++------ contracts/provider/vault/src/error.rs | 5 +- contracts/provider/vault/src/msg.rs | 3 +- contracts/provider/vault/src/txs.rs | 39 ++++-------- 4 files changed, 76 insertions(+), 48 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 1f98f1e5..bb5a5161 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -12,6 +12,7 @@ use mesh_apis::local_staking_api::{ }; use mesh_apis::vault_api::{self, VaultApi}; use mesh_sync::Lockable; +use mesh_sync::Tx::InFlightStaking; use sylvia::types::{ExecCtx, InstantiateCtx, QueryCtx, ReplyCtx}; use sylvia::{contract, schemars}; @@ -21,7 +22,7 @@ use crate::msg::{ AllTxsResponse, AllTxsResponseItem, ConfigResponse, LienInfo, StakingInitInfo, TxResponse, }; use crate::state::{Config, Lien, LocalStaking, UserInfo}; -use crate::txs::{Tx, TxType, Txs}; +use crate::txs::Txs; pub const CONTRACT_NAME: &str = env!("CARGO_PKG_NAME"); pub const CONTRACT_VERSION: &str = env!("CARGO_PKG_VERSION"); @@ -520,9 +521,8 @@ impl VaultContract<'_> { // Create new tx let tx_id = self.next_tx_id(ctx.deps.storage)?; - let new_tx = Tx { + let new_tx = InFlightStaking { id: tx_id, - ty: TxType::InFlightStaking, amount, slashable, user: ctx.info.sender.clone(), @@ -540,27 +540,45 @@ impl VaultContract<'_> { // Load tx let tx = self.pending.txs.load(ctx.deps.storage, tx_id)?; - // Verify tx comes from the right contract + // Verify tx comes from the right contract, and is the right type ensure!( - tx.lienholder == ctx.info.sender, - ContractError::WrongContractTx(tx_id, ctx.info.sender.clone()) + match tx.clone() { + InFlightStaking { lienholder, .. } => { + ensure!( + lienholder == ctx.info.sender, + ContractError::WrongContractTx(tx_id, ctx.info.sender.clone()) + ); + true + } + }, + ContractError::WrongTypeTx(tx_id, tx) ); + let (_tx_amount, _tx_slashable, tx_user, tx_lienholder) = match tx { + InFlightStaking { + amount, + slashable, + user, + lienholder, + .. + } => (amount, slashable, user, lienholder), + }; + // Load lien let mut lien_lock = self .liens - .load(ctx.deps.storage, (&tx.user, &tx.lienholder))?; + .load(ctx.deps.storage, (&tx_user, &tx_lienholder))?; // Unlock it lien_lock.unlock_write()?; // Save it self.liens - .save(ctx.deps.storage, (&tx.user, &tx.lienholder), &lien_lock)?; + .save(ctx.deps.storage, (&tx_user, &tx_lienholder), &lien_lock)?; // Load user - let mut user_lock = self.users.load(ctx.deps.storage, &tx.user)?; + let mut user_lock = self.users.load(ctx.deps.storage, &tx_user)?; // Unlock it user_lock.unlock_write()?; // Save it - self.users.save(ctx.deps.storage, &tx.user, &user_lock)?; + self.users.save(ctx.deps.storage, &tx_user, &user_lock)?; // Remove tx self.pending.txs.remove(ctx.deps.storage, tx_id)?; @@ -572,26 +590,45 @@ impl VaultContract<'_> { fn rollback_stake(&self, ctx: &mut ExecCtx, tx_id: u64) -> Result<(), ContractError> { // Load tx let tx = self.pending.txs.load(ctx.deps.storage, tx_id)?; - // Verify tx comes from the right contract + + // Verify tx comes from the right contract, and is the right type ensure!( - tx.lienholder == ctx.info.sender, - ContractError::WrongContractTx(tx_id, ctx.info.sender.clone()) + match tx.clone() { + InFlightStaking { lienholder, .. } => { + ensure!( + lienholder == ctx.info.sender, + ContractError::WrongContractTx(tx_id, ctx.info.sender.clone()) + ); + true + } + }, + ContractError::WrongTypeTx(tx_id, tx) ); + let (tx_amount, tx_slashable, tx_user, tx_lienholder) = match tx { + InFlightStaking { + amount, + slashable, + user, + lienholder, + .. + } => (amount, slashable, user, lienholder), + }; + // Load lien let mut lien_lock = self .liens - .load(ctx.deps.storage, (&tx.user, &tx.lienholder))?; + .load(ctx.deps.storage, (&tx_user, &tx_lienholder))?; // Rollback amount (need to unlock it first) lien_lock.unlock_write()?; let lien = lien_lock.write()?; - lien.amount -= tx.amount; + lien.amount -= tx_amount; // Save it unlocked self.liens - .save(ctx.deps.storage, (&tx.user, &tx.lienholder), &lien_lock)?; + .save(ctx.deps.storage, (&tx_user, &tx_lienholder), &lien_lock)?; // Load user - let mut user_lock = self.users.load(ctx.deps.storage, &tx.user)?; + let mut user_lock = self.users.load(ctx.deps.storage, &tx_user)?; // Rollback user's max_lien (need to unlock it first) user_lock.unlock_write()?; let mut user = user_lock.write()?; @@ -600,7 +637,7 @@ impl VaultContract<'_> { // is already written to storage user.max_lien = self .liens - .prefix(&tx.user) + .prefix(&tx_user) .range(ctx.deps.storage, None, None, Order::Ascending) .try_fold(Uint128::zero(), |max_lien, item| { let (_, lien_lock) = item?; @@ -609,8 +646,8 @@ impl VaultContract<'_> { lien.map(|lien| max_lien.max(lien.amount)) })?; - user.total_slashable -= tx.amount * tx.slashable; - self.users.save(ctx.deps.storage, &tx.user, &user_lock)?; + user.total_slashable -= tx_amount * tx_slashable; + self.users.save(ctx.deps.storage, &tx_user, &user_lock)?; // Remove tx self.pending.txs.remove(ctx.deps.storage, tx_id)?; diff --git a/contracts/provider/vault/src/error.rs b/contracts/provider/vault/src/error.rs index 5f56b09f..4d79808b 100644 --- a/contracts/provider/vault/src/error.rs +++ b/contracts/provider/vault/src/error.rs @@ -1,6 +1,6 @@ use cosmwasm_std::{Addr, StdError, Uint128}; use cw_utils::{ParseReplyError, PaymentError}; -use mesh_sync::LockError; +use mesh_sync::{LockError, Tx}; use thiserror::Error; #[derive(Error, Debug, PartialEq)] @@ -38,6 +38,9 @@ pub enum ContractError { #[error("Invalid reply id: {0}")] InvalidReplyId(u64), + #[error("The tx {0} exists but is of the wrong type: {1}")] + WrongTypeTx(u64, Tx), + #[error("The tx {0} exists but comes from the wrong address: {1}")] WrongContractTx(u64, Addr), } diff --git a/contracts/provider/vault/src/msg.rs b/contracts/provider/vault/src/msg.rs index 02c225b8..4a5e3871 100644 --- a/contracts/provider/vault/src/msg.rs +++ b/contracts/provider/vault/src/msg.rs @@ -1,5 +1,6 @@ use cosmwasm_schema::cw_serde; use cosmwasm_std::{Binary, Uint128}; +use mesh_sync::Tx; /// This is the info used to construct the native staking contract #[cw_serde] @@ -53,7 +54,7 @@ pub struct ConfigResponse { pub local_staking: String, } -pub type TxResponse = crate::txs::Tx; +pub type TxResponse = Tx; pub type AllTxsResponseItem = TxResponse; #[cw_serde] diff --git a/contracts/provider/vault/src/txs.rs b/contracts/provider/vault/src/txs.rs index 734330cf..82ffa79a 100644 --- a/contracts/provider/vault/src/txs.rs +++ b/contracts/provider/vault/src/txs.rs @@ -1,29 +1,7 @@ -use cosmwasm_schema::cw_serde; -use cosmwasm_std::{Addr, Decimal, Order, StdResult, Storage, Uint128}; +use cosmwasm_std::{Addr, Order, StdResult, Storage}; use cw_storage_plus::{Index, IndexList, IndexedMap, MultiIndex}; - -#[cw_serde] -pub enum TxType { - InFlightStaking, - // TODO - // Slash, -} - -#[cw_serde] -pub struct Tx { - /// Transaction id - pub id: u64, - /// Transaction type - pub ty: TxType, - /// Associated amount - pub amount: Uint128, - /// Slashable portion of lien - pub slashable: Decimal, - /// Associated user - pub user: Addr, - /// Remote staking contract - pub lienholder: Addr, -} +use mesh_sync::Tx; +use mesh_sync::Tx::InFlightStaking; pub struct TxIndexes<'a> { // Last type param defines the pk deserialization type @@ -44,7 +22,16 @@ pub struct Txs<'a> { impl<'a> Txs<'a> { pub fn new(storage_key: &'a str, user_subkey: &'a str) -> Self { let indexes = TxIndexes { - users: MultiIndex::new(|_, tx| tx.user.clone(), storage_key, user_subkey), + users: MultiIndex::new( + |_, tx| { + let user = match tx { + InFlightStaking { user, .. } => user, + }; + user.clone() + }, + storage_key, + user_subkey, + ), }; let txs = IndexedMap::new(storage_key, indexes); From 7d8b2686632cf2c750998eec63ca4c9acebfee7c Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:23:25 +0200 Subject: [PATCH 40/47] Adapt vault tests to new Tx format --- contracts/provider/vault/src/multitest.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index d248b239..cbcbfa74 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -9,7 +9,8 @@ use crate::{contract, msg::AccountResponse}; use cosmwasm_std::StdError::GenericErr; use cosmwasm_std::{coin, coins, to_binary, Addr, Binary, Decimal, Empty, Uint128}; use cw_multi_test::App as MtApp; -use mesh_sync::LockError; +use mesh_sync::Tx::InFlightStaking; +use mesh_sync::{LockError, Tx}; use sylvia::multitest::App; const OSMO: &str = "OSMO"; @@ -448,7 +449,9 @@ fn stake_local() { #[track_caller] fn get_last_pending_tx_id(vault: &VaultContractProxy) -> Option { let txs = vault.all_pending_txs(None, None).unwrap().txs; - txs.first().map(|tx| tx.id) + txs.first().map(|tx| match tx { + Tx::InFlightStaking { id, .. } => *id, + }) } #[test] @@ -880,10 +883,10 @@ fn stake_cross_txs() { .unwrap(); // First tx is still pending - assert_eq!( - vault.all_pending_txs(None, None).unwrap().txs[0].id, - first_tx - ); + let first_id = match vault.all_pending_txs(None, None).unwrap().txs[0] { + InFlightStaking { id, .. } => id, + }; + assert_eq!(first_id, first_tx); // Cannot query account while pending assert!(matches!( From 8fd2445d94df913916ee7009bf62979ce8b072d6 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:24:12 +0200 Subject: [PATCH 41/47] Adapt external-staking tests to new Tx format --- Cargo.lock | 1 + contracts/provider/external-staking/Cargo.toml | 1 + contracts/provider/external-staking/src/multitest.rs | 10 +++++++--- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 921ceda4..9b4eb084 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -399,6 +399,7 @@ dependencies = [ "mesh-apis", "mesh-native-staking", "mesh-native-staking-proxy", + "mesh-sync", "mesh-vault", "schemars", "serde", diff --git a/contracts/provider/external-staking/Cargo.toml b/contracts/provider/external-staking/Cargo.toml index 12c4591f..424ddbfc 100644 --- a/contracts/provider/external-staking/Cargo.toml +++ b/contracts/provider/external-staking/Cargo.toml @@ -39,6 +39,7 @@ anyhow = { workspace = true } mesh-vault = { workspace = true, features = ["mt"] } mesh-native-staking-proxy = { workspace = true, features = ["mt"] } mesh-native-staking = { workspace = true, features = ["mt"] } +mesh-sync = { workspace = true } [[bin]] name = "schema" diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index 0d57f8d2..c93a0d80 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -1,3 +1,5 @@ +use anyhow::Result as AnyResult; + use cosmwasm_std::{coin, coins, to_binary, Addr, Decimal}; use mesh_native_staking::contract::multitest_utils::CodeId as NativeStakingCodeId; use mesh_native_staking::contract::InstantiateMsg as NativeStakingInstantiateMsg; @@ -6,6 +8,8 @@ use mesh_vault::contract::multitest_utils::{CodeId as VaultCodeId, VaultContract use mesh_vault::contract::test_utils::VaultApi; use mesh_vault::msg::StakingInitInfo; +use mesh_sync::Tx; + use cw_multi_test::App as MtApp; use sylvia::multitest::App; @@ -14,8 +18,6 @@ use crate::contract::multitest_utils::{CodeId, ExternalStakingContractProxy}; use crate::error::ContractError; use crate::msg::{ReceiveVirtualStake, StakeInfo}; -use anyhow::Result as AnyResult; - const OSMO: &str = "osmo"; const STAR: &str = "star"; @@ -285,7 +287,9 @@ fn staking() { #[track_caller] fn get_last_pending_tx_id(vault: &VaultContractProxy) -> Option { let txs = vault.all_pending_txs(None, None).unwrap().txs; - txs.first().map(|tx| tx.id) + txs.first().map(|tx| match tx { + Tx::InFlightStaking { id, .. } => *id, + }) } #[test] From 5ca9e4ad0e04f4e7e372beb32e9cb1a86db5a3db Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:29:49 +0200 Subject: [PATCH 42/47] Add FIXME --- contracts/provider/vault/src/contract.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index bb5a5161..a2c6fa43 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -370,6 +370,7 @@ impl VaultContract<'_> { account_lock .read() .map(|account| !with_collateral || !account.collateral.is_zero()) // Skip zero collateral + // FIXME: Don't skip write-locked accounts (map to `MaybeAccount` wrapper) .unwrap_or(false) // Skip write-locked accounts }) .unwrap_or(false) // Skip other errors From a0d54cf94f069a397d9147a7491d29878ca376e1 Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:40:39 +0200 Subject: [PATCH 43/47] Refactor max lien recalculation --- contracts/provider/vault/src/contract.rs | 42 ++++++++++++++---------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index a2c6fa43..d9ad08ff 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -636,22 +636,36 @@ impl VaultContract<'_> { // Max lien has to be recalculated from scratch; the just rolled back lien // is already written to storage - user.max_lien = self + self.recalculate_max_lien(ctx.deps.storage, &tx_user, &mut user)?; + + user.total_slashable -= tx_amount * tx_slashable; + self.users.save(ctx.deps.storage, &tx_user, &user_lock)?; + + // Remove tx + self.pending.txs.remove(ctx.deps.storage, tx_id)?; + Ok(()) + } + + /// Recalculates the max lien for the user + /// + /// Needs to be called with the liens belonging to the user in an unlocked state, so that they + /// can be read. + fn recalculate_max_lien( + &self, + storage: &mut dyn Storage, + user: &Addr, + user_info: &mut UserInfo, + ) -> Result<(), ContractError> { + user_info.max_lien = self .liens - .prefix(&tx_user) - .range(ctx.deps.storage, None, None, Order::Ascending) + .prefix(&user) + .range(storage, None, None, Order::Ascending) .try_fold(Uint128::zero(), |max_lien, item| { let (_, lien_lock) = item?; // Shouldn't fail, because unlocked already let lien = lien_lock.read().map_err(Into::::into); lien.map(|lien| max_lien.max(lien.amount)) })?; - - user.total_slashable -= tx_amount * tx_slashable; - self.users.save(ctx.deps.storage, &tx_user, &user_lock)?; - - // Remove tx - self.pending.txs.remove(ctx.deps.storage, tx_id)?; Ok(()) } @@ -684,15 +698,7 @@ impl VaultContract<'_> { // Max lien has to be recalculated from scratch; the just saved lien // is already written to storage - user.max_lien = self - .liens - .prefix(&owner) - .range(ctx.deps.storage, None, None, Order::Ascending) - .try_fold(Uint128::zero(), |max_lien, item| { - let (_, lien_lock) = item?; - let lien = lien_lock.read().map_err(Into::::into); - lien.map(|lien| max_lien.max(lien.amount)) - })?; + self.recalculate_max_lien(ctx.deps.storage, &owner, &mut user)?; user.total_slashable -= amount * slashable; self.users.save(ctx.deps.storage, &owner, &user_lock)?; From 12fb25db3d7eb215f1db9a923af10a89d08126ca Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:41:45 +0200 Subject: [PATCH 44/47] write access lien directly --- contracts/provider/vault/src/contract.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index d9ad08ff..d63fdafb 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -683,11 +683,10 @@ impl VaultContract<'_> { .liens .may_load(ctx.deps.storage, (&owner, &ctx.info.sender))? .ok_or(ContractError::UnknownLienholder)?; - let lien = lien_lock.read()?; + let lien = lien_lock.write()?; ensure!(lien.amount >= amount, ContractError::InsufficientLien); let slashable = lien.slashable; - let lien = lien_lock.write()?; lien.amount -= amount; self.liens From d8c98f4918654b71d4f96ecf729d7787bae5702f Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:48:16 +0200 Subject: [PATCH 45/47] Refactor / rename pending txs query for clarity --- contracts/provider/vault/src/contract.rs | 2 +- contracts/provider/vault/src/multitest.rs | 18 +++++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index d63fdafb..17b50d52 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -406,7 +406,7 @@ impl VaultContract<'_> { /// Reports txs in descending order (newest first). /// `start_after` is the last tx id included in previous page #[msg(query)] - fn all_pending_txs( + fn all_pending_txs_desc( &self, ctx: QueryCtx, start_after: Option, diff --git a/contracts/provider/vault/src/multitest.rs b/contracts/provider/vault/src/multitest.rs index cbcbfa74..6e1c7982 100644 --- a/contracts/provider/vault/src/multitest.rs +++ b/contracts/provider/vault/src/multitest.rs @@ -448,7 +448,7 @@ fn stake_local() { #[track_caller] fn get_last_pending_tx_id(vault: &VaultContractProxy) -> Option { - let txs = vault.all_pending_txs(None, None).unwrap().txs; + let txs = vault.all_pending_txs_desc(None, None).unwrap().txs; txs.first().map(|tx| match tx { Tx::InFlightStaking { id, .. } => *id, }) @@ -827,7 +827,7 @@ fn stake_cross_txs() { ); // No pending txs - assert_eq!(vault.all_pending_txs(None, None).unwrap().txs, vec![]); + assert_eq!(vault.all_pending_txs_desc(None, None).unwrap().txs, vec![]); // Can query all accounts let accounts = vault.all_accounts(false, None, None).unwrap(); assert_eq!(accounts.accounts.len(), 2); @@ -844,7 +844,7 @@ fn stake_cross_txs() { .unwrap(); // One pending tx - assert_eq!(vault.all_pending_txs(None, None).unwrap().txs.len(), 1); + assert_eq!(vault.all_pending_txs_desc(None, None).unwrap().txs.len(), 1); // Same user cannot stake while pending tx assert_eq!( @@ -872,7 +872,7 @@ fn stake_cross_txs() { .unwrap(); // Two pending txs - assert_eq!(vault.all_pending_txs(None, None).unwrap().txs.len(), 2); + assert_eq!(vault.all_pending_txs_desc(None, None).unwrap().txs.len(), 2); // Last tx commit_tx call let last_tx = get_last_pending_tx_id(&vault).unwrap(); @@ -883,7 +883,7 @@ fn stake_cross_txs() { .unwrap(); // First tx is still pending - let first_id = match vault.all_pending_txs(None, None).unwrap().txs[0] { + let first_id = match vault.all_pending_txs_desc(None, None).unwrap().txs[0] { InFlightStaking { id, .. } => id, }; assert_eq!(first_id, first_tx); @@ -1041,7 +1041,7 @@ fn stake_cross_rollback_tx() { .unwrap(); // One pending tx - assert_eq!(vault.all_pending_txs(None, None).unwrap().txs.len(), 1); + assert_eq!(vault.all_pending_txs_desc(None, None).unwrap().txs.len(), 1); // Rollback tx let last_tx = get_last_pending_tx_id(&vault).unwrap(); @@ -1052,7 +1052,11 @@ fn stake_cross_rollback_tx() { .unwrap(); // No pending txs - assert!(vault.all_pending_txs(None, None).unwrap().txs.is_empty()); + assert!(vault + .all_pending_txs_desc(None, None) + .unwrap() + .txs + .is_empty()); // Funds are restored let acc = vault.account(user.to_owned()).unwrap(); From b0c52d267aebcf5f8c4904a17950f50f0505b9ed Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:49:09 +0200 Subject: [PATCH 46/47] Fix clippy warnings --- contracts/provider/vault/src/contract.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/contracts/provider/vault/src/contract.rs b/contracts/provider/vault/src/contract.rs index 17b50d52..ed5da39c 100644 --- a/contracts/provider/vault/src/contract.rs +++ b/contracts/provider/vault/src/contract.rs @@ -632,11 +632,11 @@ impl VaultContract<'_> { let mut user_lock = self.users.load(ctx.deps.storage, &tx_user)?; // Rollback user's max_lien (need to unlock it first) user_lock.unlock_write()?; - let mut user = user_lock.write()?; + let user = user_lock.write()?; // Max lien has to be recalculated from scratch; the just rolled back lien // is already written to storage - self.recalculate_max_lien(ctx.deps.storage, &tx_user, &mut user)?; + self.recalculate_max_lien(ctx.deps.storage, &tx_user, user)?; user.total_slashable -= tx_amount * tx_slashable; self.users.save(ctx.deps.storage, &tx_user, &user_lock)?; @@ -658,7 +658,7 @@ impl VaultContract<'_> { ) -> Result<(), ContractError> { user_info.max_lien = self .liens - .prefix(&user) + .prefix(user) .range(storage, None, None, Order::Ascending) .try_fold(Uint128::zero(), |max_lien, item| { let (_, lien_lock) = item?; @@ -693,11 +693,11 @@ impl VaultContract<'_> { .save(ctx.deps.storage, (&owner, &ctx.info.sender), &lien_lock)?; let mut user_lock = self.users.load(ctx.deps.storage, &owner)?; - let mut user = user_lock.write()?; + let user = user_lock.write()?; // Max lien has to be recalculated from scratch; the just saved lien // is already written to storage - self.recalculate_max_lien(ctx.deps.storage, &owner, &mut user)?; + self.recalculate_max_lien(ctx.deps.storage, &owner, user)?; user.total_slashable -= amount * slashable; self.users.save(ctx.deps.storage, &owner, &user_lock)?; From 4fcc33785e6206f2569519d4e4c9da79f716cf4f Mon Sep 17 00:00:00 2001 From: Mauro Lacy Date: Wed, 21 Jun 2023 09:54:32 +0200 Subject: [PATCH 47/47] Fix: pending txs renaming --- contracts/provider/external-staking/src/multitest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/provider/external-staking/src/multitest.rs b/contracts/provider/external-staking/src/multitest.rs index c93a0d80..3dff0f15 100644 --- a/contracts/provider/external-staking/src/multitest.rs +++ b/contracts/provider/external-staking/src/multitest.rs @@ -286,7 +286,7 @@ fn staking() { #[track_caller] fn get_last_pending_tx_id(vault: &VaultContractProxy) -> Option { - let txs = vault.all_pending_txs(None, None).unwrap().txs; + let txs = vault.all_pending_txs_desc(None, None).unwrap().txs; txs.first().map(|tx| match tx { Tx::InFlightStaking { id, .. } => *id, })