Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add transactional helper #357

Merged
merged 5 commits into from
Jul 28, 2021
Merged

Add transactional helper #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
54b49fb
Add transactional helper
ethanfrey Jul 27, 2021
bd72554
Use transactional in wasm keeper
ethanfrey Jul 27, 2021
185d604
Add comment
ethanfrey Jul 27, 2021
318bfe3
Remove unused rollback method
ethanfrey Jul 28, 2021
ddc3223
Remove use of test-only cache and use transactional in all tests
ethanfrey Jul 28, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 26 additions & 37 deletions packages/multi-test/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use serde::Serialize;
use crate::bank::Bank;
use crate::contracts::Contract;
use crate::executor::{AppResponse, Executor};
use crate::transactions::StorageTransaction;
use crate::transactions::transactional;
use crate::wasm::{Wasm, WasmKeeper};

pub fn next_block(block: &mut BlockInfo) {
Expand Down Expand Up @@ -106,22 +106,12 @@ where
// meaning, wrap current state, all writes go to a cache, only when execute
// returns a success do we flush it (otherwise drop it)

let mut cache = StorageTransaction::new(self.storage.as_ref());

// run all messages, stops at first error
let res: Result<Vec<AppResponse>, String> = msgs
.into_iter()
.map(|msg| {
self.router
.execute(&mut cache, &self.block, sender.clone(), msg)
})
.collect();

// this only happens if all messages run successfully
if res.is_ok() {
cache.prepare().commit(self.storage.as_mut())
}
res
let (block, router) = (&self.block, &self.router);
transactional(self.storage.as_mut(), |write_cache, _| {
msgs.into_iter()
.map(|msg| router.execute(write_cache, block, sender.clone(), msg))
.collect()
})
}

/// This is an "admin" function to let us adjust bank accounts
Expand Down Expand Up @@ -277,6 +267,7 @@ mod test {
PayoutCountResponse, PayoutInitMessage, PayoutQueryMsg, PayoutSudoMsg, ReflectMessage,
ReflectQueryMsg,
};
use crate::transactions::StorageTransaction;
use crate::BankKeeper;

use super::*;
Expand Down Expand Up @@ -745,28 +736,26 @@ mod test {
assert_eq!(router_rcpt, vec![]);

// now, second level cache
let mut cache2 = cache.cache();
let msg = BankMsg::Send {
to_address: rcpt.clone().into(),
amount: coins(12, "eth"),
};
app.router
.execute(&mut cache2, &app.block, owner, msg.into())
.unwrap();

// shows up in 2nd cache
let cached_rcpt = query_router(&app.router, &cache, &rcpt);
assert_eq!(coins(25, "eth"), cached_rcpt);
let cached2_rcpt = query_router(&app.router, &cache2, &rcpt);
assert_eq!(coins(37, "eth"), cached2_rcpt);

// apply second to first
let ops = cache2.prepare();
ops.commit(&mut cache);
transactional(&mut cache, |cache2, read| {
let msg = BankMsg::Send {
to_address: rcpt.clone().into(),
amount: coins(12, "eth"),
};
app.router
.execute(cache2, &app.block, owner, msg.into())
.unwrap();

// shows up in 2nd cache
let cached_rcpt = query_router(&app.router, read, &rcpt);
assert_eq!(coins(25, "eth"), cached_rcpt);
let cached2_rcpt = query_router(&app.router, cache2, &rcpt);
assert_eq!(coins(37, "eth"), cached2_rcpt);
Ok(())
})
.unwrap();

// apply first to router
let ops = cache.prepare();
ops.commit(app.storage.as_mut());
cache.prepare().commit(app.storage.as_mut());

let committed = query_app(&app, &rcpt);
assert_eq!(coins(37, "eth"), committed);
Expand Down
32 changes: 10 additions & 22 deletions packages/multi-test/src/transactions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@ use cosmwasm_std::{Order, Pair};
/// This is internal as it can change any time if the map implementation is swapped out.
type BTreeMapPairRef<'a, T = Vec<u8>> = (&'a Vec<u8>, &'a T);

pub fn transactional<F, T>(base: &mut dyn Storage, action: F) -> Result<T, String>
where
F: FnOnce(&mut dyn Storage, &dyn Storage) -> Result<T, String>,
{
let mut cache = StorageTransaction::new(base);
let res = action(&mut cache, base)?;
cache.prepare().commit(base);
Ok(res)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice helper / pattern.


pub struct StorageTransaction<'a> {
/// read-only access to backing storage
storage: &'a dyn Storage,
Expand All @@ -35,19 +45,10 @@ impl<'a> StorageTransaction<'a> {
}
}

#[allow(dead_code)]
pub fn cache(&self) -> StorageTransaction {
StorageTransaction::new(self)
}

/// prepares this transaction to be committed to storage
pub fn prepare(self) -> RepLog {
self.rep_log
}

/// rollback will consume the checkpoint and drop all changes (not really needed, going out of scope does the same, but nice for clarity)
#[allow(dead_code)]
pub fn rollback(self) {}
}

impl<'a> Storage for StorageTransaction<'a> {
Expand Down Expand Up @@ -572,19 +573,6 @@ mod test {
assert_eq!(base.get(b"subtx"), Some(b"works".to_vec()));
}

#[test]
fn rollback_has_no_effect() {
let mut base = MemoryStorage::new();
base.set(b"foo", b"bar");

let mut check = StorageTransaction::new(&base);
assert_eq!(check.get(b"foo"), Some(b"bar".to_vec()));
check.set(b"subtx", b"works");
check.rollback();

assert_eq!(base.get(b"subtx"), None);
}

#[test]
fn ignore_same_as_rollback() {
let mut base = MemoryStorage::new();
Expand Down
174 changes: 92 additions & 82 deletions packages/multi-test/src/wasm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use cw_storage_plus::Map;
use crate::app::{Router, RouterQuerier};
use crate::contracts::Contract;
use crate::executor::AppResponse;
use crate::transactions::StorageTransaction;
use crate::transactions::transactional;

// Contract state is kept in Storage, separate from the contracts themselves
const CONTRACTS: Map<&Addr, ContractData> = Map::new("contracts");
Expand Down Expand Up @@ -287,19 +287,21 @@ where
contract: Addr,
msg: SubMsg<C>,
) -> Result<AppResponse, String> {
let SubMsg {
msg, id, reply_on, ..
} = msg;

// execute in cache
let mut cache = StorageTransaction::new(storage);
let res = router.execute(&mut cache, block, contract.clone(), msg.msg);
if res.is_ok() {
cache.prepare().commit(storage);
}
let res = transactional(storage, |write_cache, _| {
router.execute(write_cache, block, contract.clone(), msg)
});

// call reply if meaningful
if let Ok(r) = res {
if matches!(msg.reply_on, ReplyOn::Always | ReplyOn::Success) {
if matches!(reply_on, ReplyOn::Always | ReplyOn::Success) {
let mut orig = r.clone();
let reply = Reply {
id: msg.id,
id,
result: ContractResult::Ok(SubMsgExecutionResponse {
events: r.events,
data: r.data,
Expand All @@ -318,9 +320,9 @@ where
Ok(r)
}
} else if let Err(e) = res {
if matches!(msg.reply_on, ReplyOn::Always | ReplyOn::Error) {
if matches!(reply_on, ReplyOn::Always | ReplyOn::Error) {
let reply = Reply {
id: msg.id,
id,
result: ContractResult::Err(e),
};
self._reply(router, storage, block, contract, reply)
Expand Down Expand Up @@ -513,23 +515,22 @@ where
.get(&contract.code_id)
.ok_or_else(|| "Unregistered code id".to_string())?;

let mut cache = StorageTransaction::new(storage);
let mut contract_storage = self.contract_storage(&mut cache, &address);
let querier = RouterQuerier::new(router, storage, block);
let env = self.get_env(address, block);

let deps = DepsMut {
storage: contract_storage.as_mut(),
api: self.api.deref(),
querier: QuerierWrapper::new(&querier),
};
let res = action(handler, deps, env)?;

// forces drop, so we can write to cache
drop(contract_storage);
// if this succeeds, commit
cache.prepare().commit(storage);
Ok(res)
// We don't actually need a transaction here, as it is already embedded in a transactional.
// execute_submsg or App.execute_multi.
// However, we need to get write and read access to the same storage in two different objects,
// and this is the only way I know how to do so.
transactional(storage, |write_cache, read_store| {
let mut contract_storage = self.contract_storage(write_cache, &address);
let querier = RouterQuerier::new(router, read_store, block);
let env = self.get_env(address, block);

let deps = DepsMut {
storage: contract_storage.as_mut(),
api: self.api.deref(),
querier: QuerierWrapper::new(&querier),
};
action(handler, deps, env)
Copy link
Contributor

Choose a reason for hiding this comment

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

Whoa, one action inside an action.

})
}

fn load_contract(&self, storage: &dyn Storage, address: &Addr) -> Result<ContractData, String> {
Expand Down Expand Up @@ -816,68 +817,77 @@ mod test {
.unwrap();
cache.prepare().commit(&mut wasm_storage);

// create a new cache and check we can use contract 1
let mut cache = StorageTransaction::new(&wasm_storage);
assert_payout(&keeper, &mut cache, &contract1, &payout1);

// create contract 2 and use it
let contract2 = keeper.register_contract(&mut cache, code_id).unwrap();
let payout2 = coin(50, "BTC");
let info = mock_info("foobar", &[]);
let init_msg = to_vec(&PayoutInitMessage {
payout: payout2.clone(),
})
.unwrap();
let _res = keeper
.call_instantiate(
contract2.clone(),
&mut cache,
&mock_router(),
&block,
info,
init_msg,
)
.unwrap();
assert_payout(&keeper, &mut cache, &contract2, &payout2);

// create a level2 cache and check we can use contract 1 and contract 2
let mut cache2 = cache.cache();
assert_payout(&keeper, &mut cache2, &contract1, &payout1);
assert_payout(&keeper, &mut cache2, &contract2, &payout2);

// create a contract on level 2
let contract3 = keeper.register_contract(&mut cache2, code_id).unwrap();
let payout3 = coin(1234, "ATOM");
let info = mock_info("johnny", &[]);
let init_msg = to_vec(&PayoutInitMessage {
payout: payout3.clone(),
})
.unwrap();
let _res = keeper
.call_instantiate(
contract3.clone(),
&mut cache2,
&mock_router(),
&block,
info,
init_msg,
)

// create a new cache and check we can use contract 1
let (contract2, contract3) = transactional(&mut wasm_storage, |cache, wasm_reader| {
assert_payout(&keeper, cache, &contract1, &payout1);

// create contract 2 and use it
let contract2 = keeper.register_contract(cache, code_id).unwrap();
let info = mock_info("foobar", &[]);
let init_msg = to_vec(&PayoutInitMessage {
payout: payout2.clone(),
})
.unwrap();
let _res = keeper
.call_instantiate(
contract2.clone(),
cache,
&mock_router(),
&block,
info,
init_msg,
)
.unwrap();
assert_payout(&keeper, cache, &contract2, &payout2);

// create a level2 cache and check we can use contract 1 and contract 2
let contract3 = transactional(cache, |cache2, read| {
assert_payout(&keeper, cache2, &contract1, &payout1);
assert_payout(&keeper, cache2, &contract2, &payout2);

// create a contract on level 2
let contract3 = keeper.register_contract(cache2, code_id).unwrap();
let info = mock_info("johnny", &[]);
let init_msg = to_vec(&PayoutInitMessage {
payout: payout3.clone(),
})
.unwrap();
let _res = keeper
.call_instantiate(
contract3.clone(),
cache2,
&mock_router(),
&block,
info,
init_msg,
)
.unwrap();
assert_payout(&keeper, cache2, &contract3, &payout3);

// ensure first cache still doesn't see this contract
assert_no_contract(read, &contract3);
Ok(contract3)
})
.unwrap();
assert_payout(&keeper, &mut cache2, &contract3, &payout3);

// ensure first cache still doesn't see this contract
assert_no_contract(&cache, &contract3);
// after applying transaction, all contracts present on cache
assert_payout(&keeper, cache, &contract1, &payout1);
assert_payout(&keeper, cache, &contract2, &payout2);
assert_payout(&keeper, cache, &contract3, &payout3);

// apply second to first, all contracts present
cache2.prepare().commit(&mut cache);
assert_payout(&keeper, &mut cache, &contract1, &payout1);
assert_payout(&keeper, &mut cache, &contract2, &payout2);
assert_payout(&keeper, &mut cache, &contract3, &payout3);
// but not yet the root router
assert_no_contract(wasm_reader, &contract1);
assert_no_contract(wasm_reader, &contract2);
assert_no_contract(wasm_reader, &contract3);

// apply to router
cache.prepare().commit(&mut wasm_storage);
Ok((contract2, contract3))
})
.unwrap();

// make new cache and see all contracts there
// ensure that it is now applied to the router
assert_payout(&keeper, &mut wasm_storage, &contract1, &payout1);
assert_payout(&keeper, &mut wasm_storage, &contract2, &payout2);
assert_payout(&keeper, &mut wasm_storage, &contract3, &payout3);
Expand Down