From 8777c22d32ca6fd2c2c7ba98d2eed92324825aa8 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Tue, 10 Jan 2023 17:54:19 +0900 Subject: [PATCH] cosmwasm: accounting: Use cw_transcode for events Use cw_transcode to ensure that event attribute values are always encoded as proper json, making it easier for clients to parse them back into structured data. This also lets us reuse the input messages for the events, reducing the number of different structs that we need to track. --- cosmwasm/Cargo.lock | 4 ++ .../contracts/wormchain-accounting/Cargo.toml | 4 +- .../wormchain-accounting/src/contract.rs | 16 ++--- .../contracts/wormchain-accounting/src/msg.rs | 2 + .../tests/modify_balance.rs | 22 ++++--- .../tests/submit_observations.rs | 30 ++++++--- .../wormchain-accounting/tests/submit_vaas.rs | 18 ++--- cosmwasm/packages/accounting/Cargo.toml | 2 + cosmwasm/packages/accounting/src/contract.rs | 66 ++++++------------- 9 files changed, 77 insertions(+), 87 deletions(-) diff --git a/cosmwasm/Cargo.lock b/cosmwasm/Cargo.lock index 9cedd5797..60c1e91e5 100644 --- a/cosmwasm/Cargo.lock +++ b/cosmwasm/Cargo.lock @@ -11,9 +11,11 @@ dependencies = [ "cosmwasm-schema", "cosmwasm-std", "cw-storage-plus", + "cw_transcode", "hex", "schemars", "serde", + "serde-json-wasm", "thiserror", ] @@ -2257,9 +2259,11 @@ dependencies = [ "cw-multi-test", "cw-storage-plus", "cw2", + "cw_transcode", "hex", "schemars", "serde", + "serde-json-wasm", "serde_wormhole", "thiserror", "tinyvec", diff --git a/cosmwasm/contracts/wormchain-accounting/Cargo.toml b/cosmwasm/contracts/wormchain-accounting/Cargo.toml index 57e783b79..ddf646b3a 100644 --- a/cosmwasm/contracts/wormchain-accounting/Cargo.toml +++ b/cosmwasm/contracts/wormchain-accounting/Cargo.toml @@ -20,8 +20,9 @@ cosmwasm-schema = "1" cosmwasm-std = "1" cosmwasm-storage = "1" cw-storage-plus = "0.13.2" +cw_transcode = "0.1.0" cw2 = "0.13.2" -hex = "0.4.3" +hex = { version = "0.4.3", features = ["serde"] } schemars = "0.8.8" serde = { version = "1.0.137", default-features = false, features = ["derive"] } serde_wormhole = "0.1.0" @@ -34,4 +35,5 @@ wormhole-core = { version = "0.1.0", features = ["schemars"] } [dev-dependencies] anyhow = { version = "1", features = ["backtrace"] } cw-multi-test = "0.13.2" +serde-json-wasm = "0.4" wormhole-bindings = { version = "0.1", features = ["fake"] } diff --git a/cosmwasm/contracts/wormchain-accounting/src/contract.rs b/cosmwasm/contracts/wormchain-accounting/src/contract.rs index 26c0c8859..63e7d2737 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/contract.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/contract.rs @@ -18,7 +18,7 @@ use tinyvec::{Array, TinyVec}; use wormhole::{ token::{Action, GovernancePacket, Message}, vaa::{self, Body, Header, Signature}, - Address, Chain, + Chain, }; use wormhole_bindings::WormholeQuery; @@ -237,17 +237,9 @@ fn handle_observation( // Now that the transfer has been committed, we don't need to keep it in the pending list. key.remove(deps.storage); - Ok(Some( - Event::new("Transfer") - .add_attribute("tx_hash", o.tx_hash.to_base64()) - .add_attribute("timestamp", o.timestamp.to_string()) - .add_attribute("nonce", o.nonce.to_string()) - .add_attribute("emitter_chain", o.emitter_chain.to_string()) - .add_attribute("emitter_address", Address(o.emitter_address).to_string()) - .add_attribute("sequence", o.sequence.to_string()) - .add_attribute("consistency_level", o.consistency_level.to_string()) - .add_attribute("payload", o.payload.to_base64()), - )) + cw_transcode::to_event(&o) + .map(Some) + .context("failed to transcode `Observation` to `Event`") } fn modify_balance( diff --git a/cosmwasm/contracts/wormchain-accounting/src/msg.rs b/cosmwasm/contracts/wormchain-accounting/src/msg.rs index 9868b8d13..2274555c4 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/msg.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/msg.rs @@ -24,6 +24,8 @@ pub struct Observation { pub emitter_chain: u16, // The address on the source chain that emitted this message. + #[serde(with = "hex")] + #[schemars(with = "String")] pub emitter_address: [u8; 32], // The sequence number of this observation. diff --git a/cosmwasm/contracts/wormchain-accounting/tests/modify_balance.rs b/cosmwasm/contracts/wormchain-accounting/tests/modify_balance.rs index 974e2efc8..e056cc3d5 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/modify_balance.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/modify_balance.rs @@ -25,14 +25,20 @@ fn simple_modify() { .modify_balance(modification, index, signatures) .unwrap(); - let evt = Event::new("wasm-ModifyBalance") - .add_attribute("sequence", m.sequence.to_string()) - .add_attribute("chain_id", m.chain_id.to_string()) - .add_attribute("token_chain", m.token_chain.to_string()) - .add_attribute("token_address", m.token_address.to_string()) - .add_attribute("kind", m.kind.to_string()) - .add_attribute("amount", m.amount) - .add_attribute("reason", m.reason.clone()); + let evt = Event::new("wasm-Modification") + .add_attribute("sequence", serde_json_wasm::to_string(&m.sequence).unwrap()) + .add_attribute("chain_id", serde_json_wasm::to_string(&m.chain_id).unwrap()) + .add_attribute( + "token_chain", + serde_json_wasm::to_string(&m.token_chain).unwrap(), + ) + .add_attribute( + "token_address", + serde_json_wasm::to_string(&m.token_address).unwrap(), + ) + .add_attribute("kind", serde_json_wasm::to_string(&m.kind).unwrap()) + .add_attribute("amount", serde_json_wasm::to_string(&m.amount).unwrap()) + .add_attribute("reason", serde_json_wasm::to_string(&m.reason).unwrap()); resp.assert_event(&evt); diff --git a/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs b/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs index bca440c67..e34dc6b71 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs @@ -776,15 +776,27 @@ fn emit_event_with_quorum() { let (o, responses) = transfer_tokens(&wh, &mut contract, key, msg, index, quorum).unwrap(); - let expected = Event::new("wasm-Transfer") - .add_attribute("tx_hash", o.tx_hash.to_base64()) - .add_attribute("timestamp", o.timestamp.to_string()) - .add_attribute("nonce", o.nonce.to_string()) - .add_attribute("emitter_chain", o.emitter_chain.to_string()) - .add_attribute("emitter_address", Address(o.emitter_address).to_string()) - .add_attribute("sequence", o.sequence.to_string()) - .add_attribute("consistency_level", o.consistency_level.to_string()) - .add_attribute("payload", o.payload.to_base64()); + let expected = Event::new("wasm-Observation") + .add_attribute("tx_hash", serde_json_wasm::to_string(&o.tx_hash).unwrap()) + .add_attribute( + "timestamp", + serde_json_wasm::to_string(&o.timestamp).unwrap(), + ) + .add_attribute("nonce", serde_json_wasm::to_string(&o.nonce).unwrap()) + .add_attribute( + "emitter_chain", + serde_json_wasm::to_string(&o.emitter_chain).unwrap(), + ) + .add_attribute( + "emitter_address", + serde_json_wasm::to_string(&hex::encode(o.emitter_address)).unwrap(), + ) + .add_attribute("sequence", serde_json_wasm::to_string(&o.sequence).unwrap()) + .add_attribute( + "consistency_level", + serde_json_wasm::to_string(&o.consistency_level).unwrap(), + ) + .add_attribute("payload", serde_json_wasm::to_string(&o.payload).unwrap()); assert_eq!(responses.len(), quorum); for (i, r) in responses.into_iter().enumerate() { diff --git a/cosmwasm/contracts/wormchain-accounting/tests/submit_vaas.rs b/cosmwasm/contracts/wormchain-accounting/tests/submit_vaas.rs index d8bf3d22c..b00ef88a3 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/submit_vaas.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/submit_vaas.rs @@ -91,12 +91,9 @@ fn basic() { assert_eq!(data, tx.data); assert_eq!(&digest[..], &*tx.digest); resp.assert_event( - &Event::new("wasm-CommitTransfer") - .add_attribute("key", key.to_string()) - .add_attribute("amount", data.amount.to_string()) - .add_attribute("token_chain", data.token_chain.to_string()) - .add_attribute("token_address", data.token_address.to_string()) - .add_attribute("recipient_chain", data.recipient_chain.to_string()), + &Event::new("wasm-Transfer") + .add_attribute("key", serde_json_wasm::to_string(&key).unwrap()) + .add_attribute("data", serde_json_wasm::to_string(&data).unwrap()), ); } } @@ -250,12 +247,9 @@ fn transfer_with_payload() { assert_eq!(data, tx.data); assert_eq!(&digest[..], &*tx.digest); resp.assert_event( - &Event::new("wasm-CommitTransfer") - .add_attribute("key", key.to_string()) - .add_attribute("amount", data.amount.to_string()) - .add_attribute("token_chain", data.token_chain.to_string()) - .add_attribute("token_address", data.token_address.to_string()) - .add_attribute("recipient_chain", data.recipient_chain.to_string()), + &Event::new("wasm-Transfer") + .add_attribute("key", serde_json_wasm::to_string(&key).unwrap()) + .add_attribute("data", serde_json_wasm::to_string(&data).unwrap()), ); } diff --git a/cosmwasm/packages/accounting/Cargo.toml b/cosmwasm/packages/accounting/Cargo.toml index fbe5481d6..515c1d7ea 100644 --- a/cosmwasm/packages/accounting/Cargo.toml +++ b/cosmwasm/packages/accounting/Cargo.toml @@ -19,6 +19,7 @@ base64 = "0.13" cosmwasm-schema = "1" cosmwasm-std = "1" cw-storage-plus = "0.13.2" +cw_transcode = "0.1.0" hex = "0.4.3" schemars = "0.8.8" serde = { version = "1.0.137", default-features = false } @@ -26,3 +27,4 @@ thiserror = "1" [dev-dependencies] anyhow = { version = "1", features = ["backtrace"] } +serde-json-wasm = "0.4" diff --git a/cosmwasm/packages/accounting/src/contract.rs b/cosmwasm/packages/accounting/src/contract.rs index 82ca47054..30a827847 100644 --- a/cosmwasm/packages/accounting/src/contract.rs +++ b/cosmwasm/packages/accounting/src/contract.rs @@ -116,12 +116,7 @@ pub fn commit_transfer(deps: DepsMut, t: Transfer) -> anyhow: .save(deps.storage, dst.key, &dst.balance) .context("failed to save updated destination account")?; - let evt = Event::new("CommitTransfer") - .add_attribute("key", t.key.to_string()) - .add_attribute("amount", t.data.amount.to_string()) - .add_attribute("token_chain", t.data.token_chain.to_string()) - .add_attribute("token_address", t.data.token_address.to_string()) - .add_attribute("recipient_chain", t.data.recipient_chain.to_string()); + let evt = cw_transcode::to_event(&t).context("failed to transcode `Transfer` to `Event`")?; TRANSFERS .save(deps.storage, t.key, &t.data) @@ -269,14 +264,7 @@ pub fn modify_balance( .save(deps.storage, msg.sequence, &msg) .context("failed to store `Modification`")?; - Ok(Event::new("ModifyBalance") - .add_attribute("sequence", msg.sequence.to_string()) - .add_attribute("chain_id", msg.chain_id.to_string()) - .add_attribute("token_chain", msg.token_chain.to_string()) - .add_attribute("token_address", msg.token_address.to_string()) - .add_attribute("kind", msg.kind.to_string()) - .add_attribute("amount", msg.amount) - .add_attribute("reason", msg.reason)) + cw_transcode::to_event(&msg).context("failed to transcode `Modification` to `Event`") } /// Query the balance for the account associated with `key`. @@ -471,22 +459,10 @@ mod tests { ); assert_eq!(tx.data.amount, *query_balance(deps.as_ref(), dst).unwrap()); - assert_eq!(evt.ty, "CommitTransfer"); - assert_eq!(evt.attributes.len(), 5); - - let attrs = evt - .attributes - .into_iter() - .map(|attr| (attr.key, attr.value)) - .collect::>(); - assert_eq!(attrs["key"], tx.key.to_string()); - assert_eq!(attrs["amount"], tx.data.amount.to_string()); - assert_eq!(attrs["token_chain"], tx.data.token_chain.to_string()); - assert_eq!(attrs["token_address"], tx.data.token_address.to_string()); - assert_eq!( - attrs["recipient_chain"], - tx.data.recipient_chain.to_string() - ); + let expected = Event::new("Transfer") + .add_attribute("key", serde_json_wasm::to_string(&tx.key).unwrap()) + .add_attribute("data", serde_json_wasm::to_string(&tx.data).unwrap()); + assert_eq!(expected, evt); assert_eq!(tx.data, query_transfer(deps.as_ref(), tx.key).unwrap()); } @@ -870,21 +846,21 @@ mod tests { assert_eq!(m, query_modification(deps.as_ref(), m.sequence).unwrap()); - assert_eq!(evt.ty, "ModifyBalance"); - assert_eq!(evt.attributes.len(), 7); - - let attrs = evt - .attributes - .into_iter() - .map(|attr| (attr.key, attr.value)) - .collect::>(); - assert_eq!(attrs["sequence"], m.sequence.to_string()); - assert_eq!(attrs["chain_id"], m.chain_id.to_string()); - assert_eq!(attrs["token_chain"], m.token_chain.to_string()); - assert_eq!(attrs["token_address"], m.token_address.to_string()); - assert_eq!(attrs["kind"], m.kind.to_string()); - assert_eq!(attrs["amount"], m.amount.to_string()); - assert_eq!(attrs["reason"], m.reason); + let expected = Event::new("Modification") + .add_attribute("sequence", serde_json_wasm::to_string(&m.sequence).unwrap()) + .add_attribute("chain_id", serde_json_wasm::to_string(&m.chain_id).unwrap()) + .add_attribute( + "token_chain", + serde_json_wasm::to_string(&m.token_chain).unwrap(), + ) + .add_attribute( + "token_address", + serde_json_wasm::to_string(&m.token_address).unwrap(), + ) + .add_attribute("kind", serde_json_wasm::to_string(&m.kind).unwrap()) + .add_attribute("amount", serde_json_wasm::to_string(&m.amount).unwrap()) + .add_attribute("reason", serde_json_wasm::to_string(&m.reason).unwrap()); + assert_eq!(expected, evt); } #[test]