From 1e3356b4c9bef453cc10df0627cadacd38af230d Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Mon, 16 Jan 2023 18:22:17 +0900 Subject: [PATCH] cosmwasm: accounting: Don't store Observation on-chain Now that we can calculate the digest of an Observation there's no need to store the whole thing on-chain. Instead only store the observation digest, tx_hash, and emitter chain (the tx_hash is necessary because it's not included in the digest and the emitter chain is used for servicing missing observation queries). When adding new observations we can check for equality by comparing the digests and tx hashes rather than comparing the whole object. This should further reduce the size of the on-chain state. --- .../schema/wormchain-accounting.json | 196 ++++-------------- .../wormchain-accounting/src/contract.rs | 25 ++- .../wormchain-accounting/src/state.rs | 33 ++- .../tests/missing_observations.rs | 3 +- .../tests/submit_observations.rs | 22 +- 5 files changed, 85 insertions(+), 194 deletions(-) diff --git a/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json b/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json index b3cc0aa5c..51a6ca760 100644 --- a/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json +++ b/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json @@ -691,23 +691,33 @@ "Data": { "type": "object", "required": [ + "digest", + "emitter_chain", "guardian_set_index", - "observation", - "signatures" + "signatures", + "tx_hash" ], "properties": { + "digest": { + "$ref": "#/definitions/Binary" + }, + "emitter_chain": { + "type": "integer", + "format": "uint16", + "minimum": 0.0 + }, "guardian_set_index": { "type": "integer", "format": "uint32", "minimum": 0.0 }, - "observation": { - "$ref": "#/definitions/Observation" - }, "signatures": { "type": "integer", "format": "uint128", "minimum": 0.0 + }, + "tx_hash": { + "$ref": "#/definitions/Binary" } }, "additionalProperties": false @@ -736,56 +746,6 @@ }, "additionalProperties": false }, - "Observation": { - "type": "object", - "required": [ - "consistency_level", - "emitter_address", - "emitter_chain", - "nonce", - "payload", - "sequence", - "timestamp", - "tx_hash" - ], - "properties": { - "consistency_level": { - "type": "integer", - "format": "uint8", - "minimum": 0.0 - }, - "emitter_address": { - "type": "string" - }, - "emitter_chain": { - "type": "integer", - "format": "uint16", - "minimum": 0.0 - }, - "nonce": { - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "payload": { - "$ref": "#/definitions/Binary" - }, - "sequence": { - "type": "integer", - "format": "uint64", - "minimum": 0.0 - }, - "timestamp": { - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "tx_hash": { - "$ref": "#/definitions/Binary" - } - }, - "additionalProperties": false - }, "PendingTransfer": { "type": "object", "required": [ @@ -957,23 +917,33 @@ "Data": { "type": "object", "required": [ + "digest", + "emitter_chain", "guardian_set_index", - "observation", - "signatures" + "signatures", + "tx_hash" ], "properties": { + "digest": { + "$ref": "#/definitions/Binary" + }, + "emitter_chain": { + "type": "integer", + "format": "uint16", + "minimum": 0.0 + }, "guardian_set_index": { "type": "integer", "format": "uint32", "minimum": 0.0 }, - "observation": { - "$ref": "#/definitions/Observation" - }, "signatures": { "type": "integer", "format": "uint128", "minimum": 0.0 + }, + "tx_hash": { + "$ref": "#/definitions/Binary" } }, "additionalProperties": false @@ -1002,56 +972,6 @@ }, "additionalProperties": false }, - "Observation": { - "type": "object", - "required": [ - "consistency_level", - "emitter_address", - "emitter_chain", - "nonce", - "payload", - "sequence", - "timestamp", - "tx_hash" - ], - "properties": { - "consistency_level": { - "type": "integer", - "format": "uint8", - "minimum": 0.0 - }, - "emitter_address": { - "type": "string" - }, - "emitter_chain": { - "type": "integer", - "format": "uint16", - "minimum": 0.0 - }, - "nonce": { - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "payload": { - "$ref": "#/definitions/Binary" - }, - "sequence": { - "type": "integer", - "format": "uint64", - "minimum": 0.0 - }, - "timestamp": { - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "tx_hash": { - "$ref": "#/definitions/Binary" - } - }, - "additionalProperties": false - }, "TokenAddress": { "type": "string" }, @@ -1297,69 +1217,29 @@ "Data": { "type": "object", "required": [ - "guardian_set_index", - "observation", - "signatures" - ], - "properties": { - "guardian_set_index": { - "type": "integer", - "format": "uint32", - "minimum": 0.0 - }, - "observation": { - "$ref": "#/definitions/Observation" - }, - "signatures": { - "type": "integer", - "format": "uint128", - "minimum": 0.0 - } - }, - "additionalProperties": false - }, - "Observation": { - "type": "object", - "required": [ - "consistency_level", - "emitter_address", + "digest", "emitter_chain", - "nonce", - "payload", - "sequence", - "timestamp", + "guardian_set_index", + "signatures", "tx_hash" ], "properties": { - "consistency_level": { - "type": "integer", - "format": "uint8", - "minimum": 0.0 - }, - "emitter_address": { - "type": "string" + "digest": { + "$ref": "#/definitions/Binary" }, "emitter_chain": { "type": "integer", "format": "uint16", "minimum": 0.0 }, - "nonce": { + "guardian_set_index": { "type": "integer", "format": "uint32", "minimum": 0.0 }, - "payload": { - "$ref": "#/definitions/Binary" - }, - "sequence": { + "signatures": { "type": "integer", - "format": "uint64", - "minimum": 0.0 - }, - "timestamp": { - "type": "integer", - "format": "uint32", + "format": "uint128", "minimum": 0.0 }, "tx_hash": { diff --git a/cosmwasm/contracts/wormchain-accounting/src/contract.rs b/cosmwasm/contracts/wormchain-accounting/src/contract.rs index e4302c265..194004a43 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/contract.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/contract.rs @@ -157,6 +157,8 @@ fn handle_observation( quorum: u32, sig: Signature, ) -> anyhow::Result<(ObservationStatus, Option)> { + let digest = o.digest().context(ContractError::ObservationDigest)?; + let digest_key = DIGESTS.key((o.emitter_chain, o.emitter_address.to_vec(), o.sequence)); let tx_key = transfer::Key::new(o.emitter_chain, o.emitter_address.into(), o.sequence); @@ -164,7 +166,6 @@ fn handle_observation( .may_load(deps.storage) .context("failed to load transfer digest")? { - let digest = o.digest().context(ContractError::ObservationDigest)?; if saved_digest != digest { bail!(ContractError::DigestMismatch); } @@ -177,13 +178,19 @@ fn handle_observation( .may_load(deps.storage) .map(Option::unwrap_or_default) .context("failed to load `PendingTransfer`")?; - let data = match pending - .iter_mut() - .find(|d| d.guardian_set_index() == guardian_set_index && d.observation() == &o) - { + let data = match pending.iter_mut().find(|d| { + d.guardian_set_index() == guardian_set_index + && d.digest() == &digest + && d.tx_hash() == &o.tx_hash + }) { Some(d) => d, None => { - pending.push(Data::new(o.clone(), guardian_set_index)); + pending.push(Data::new( + digest.clone(), + o.tx_hash.clone(), + o.emitter_chain, + guardian_set_index, + )); let back = pending.len() - 1; &mut pending[back] } @@ -245,7 +252,6 @@ fn handle_observation( // Save the digest of the observation so that we can check for duplicate transfer keys with // mismatched data. - let digest = o.digest().context(ContractError::ObservationDigest)?; digest_key .save(deps.storage, &digest) .context("failed to save transfer digest")?; @@ -661,10 +667,9 @@ fn query_missing_observations( let (_, v) = pending?; for data in v { if data.guardian_set_index() == guardian_set && !data.has_signature(index) { - let o = data.observation(); missing.push(MissingObservation { - chain_id: o.emitter_chain, - tx_hash: o.tx_hash.clone(), + chain_id: data.emitter_chain(), + tx_hash: data.tx_hash().clone(), }); } } diff --git a/cosmwasm/contracts/wormchain-accounting/src/state.rs b/cosmwasm/contracts/wormchain-accounting/src/state.rs index f1b4563eb..33309b8f8 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/state.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/state.rs @@ -4,8 +4,6 @@ use cosmwasm_std::Binary; use cw_storage_plus::Map; use tinyvec::TinyVec; -use crate::msg::Observation; - pub const PENDING_TRANSFERS: Map> = Map::new("pending_transfers"); pub const CHAIN_REGISTRATIONS: Map = Map::new("chain_registrations"); pub const DIGESTS: Map<(u16, Vec, u64), Binary> = Map::new("digests"); @@ -19,22 +17,39 @@ pub struct PendingTransfer { #[cw_serde] #[derive(Default)] pub struct Data { - observation: Observation, - guardian_set_index: u32, + digest: Binary, + tx_hash: Binary, signatures: u128, + guardian_set_index: u32, + emitter_chain: u16, } impl Data { - pub const fn new(observation: Observation, guardian_set_index: u32) -> Self { + pub const fn new( + digest: Binary, + tx_hash: Binary, + emitter_chain: u16, + guardian_set_index: u32, + ) -> Self { Self { - observation, - guardian_set_index, + digest, + tx_hash, signatures: 0, + guardian_set_index, + emitter_chain, } } - pub fn observation(&self) -> &Observation { - &self.observation + pub fn digest(&self) -> &Binary { + &self.digest + } + + pub fn tx_hash(&self) -> &Binary { + &self.tx_hash + } + + pub fn emitter_chain(&self) -> u16 { + self.emitter_chain } pub fn guardian_set_index(&self) -> u32 { diff --git a/cosmwasm/contracts/wormchain-accounting/tests/missing_observations.rs b/cosmwasm/contracts/wormchain-accounting/tests/missing_observations.rs index 6be2be8bb..3ffda8bc7 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/missing_observations.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/missing_observations.rs @@ -43,6 +43,7 @@ fn missing_observations() { .unwrap() as usize; let o = create_observation(); + let digest = o.digest().unwrap(); let data = to_binary(&[o.clone()]).unwrap(); let signatures = wh.sign(&data); @@ -56,7 +57,7 @@ fn missing_observations() { // The transfer should still be pending. let key = transfer::Key::new(o.emitter_chain, o.emitter_address.into(), o.sequence); let pending = contract.query_pending_transfer(key).unwrap(); - assert_eq!(&o, pending[0].observation()); + assert_eq!(&digest, pending[0].digest()); for (i, s) in signatures.iter().enumerate() { let resp = contract.query_missing_observations(index, s.index).unwrap(); diff --git a/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs b/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs index 99f26e09f..21bf240ef 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs @@ -75,7 +75,8 @@ fn batch() { let key = transfer::Key::new(o.emitter_chain, o.emitter_address.into(), o.sequence); let data = contract.query_pending_transfer(key.clone()).unwrap(); - assert_eq!(o, data[0].observation()); + let digest = o.digest().unwrap(); + assert_eq!(&digest, data[0].digest()); // Make sure the transfer hasn't yet been committed. assert!(matches!(status[&key], ObservationStatus::Pending)); @@ -436,23 +437,12 @@ fn no_quorum() { fee: Amount([0u8; 32]), }; - transfer_tokens( - &wh, - &mut contract, - key.clone(), - msg.clone(), - index, - quorum - 1, - ) - .unwrap(); + let (o, _) = transfer_tokens(&wh, &mut contract, key.clone(), msg, index, quorum - 1).unwrap(); let data = contract.query_pending_transfer(key.clone()).unwrap(); - assert_eq!(emitter_chain, data[0].observation().emitter_chain); - assert_eq!(emitter_address, data[0].observation().emitter_address); - assert_eq!(sequence, data[0].observation().sequence); - - let actual = serde_wormhole::from_slice(&data[0].observation().payload).unwrap(); - assert_eq!(msg, actual); + assert_eq!(emitter_chain, data[0].emitter_chain()); + assert_eq!(&o.digest().unwrap(), data[0].digest()); + assert_eq!(&o.tx_hash, data[0].tx_hash()); // Make sure the transfer hasn't yet been committed. contract