From b1050f69ee30265bf2fa9daa6246d62760f572cd Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Thu, 12 Jan 2023 16:41:26 +0900 Subject: [PATCH] cosmwasm: accounting: Simply transfer queries Rather than forcing clients to guess whether a transfer is pending or committed use a single `TransferStatus` query that will return whether the transfer is still pending or already committed. This will make it easier for clients to keep the pending and committed transfer state in sync to avoid unnecessary overhead. --- .../schema/wormchain-accounting.json | 423 +++++++++++++----- .../wormchain-accounting/src/contract.rs | 80 ++-- .../contracts/wormchain-accounting/src/msg.rs | 37 +- .../wormchain-accounting/tests/helpers/mod.rs | 48 +- .../wormchain-accounting/tests/query.rs | 19 + 5 files changed, 451 insertions(+), 156 deletions(-) diff --git a/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json b/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json index 4966e4817..42fb12e65 100644 --- a/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json +++ b/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json @@ -221,18 +221,6 @@ }, "additionalProperties": false }, - { - "type": "object", - "required": [ - "transfer" - ], - "properties": { - "transfer": { - "$ref": "#/definitions/Key" - } - }, - "additionalProperties": false - }, { "type": "object", "required": [ @@ -266,18 +254,6 @@ }, "additionalProperties": false }, - { - "type": "object", - "required": [ - "pending_transfer" - ], - "properties": { - "pending_transfer": { - "$ref": "#/definitions/Key" - } - }, - "additionalProperties": false - }, { "type": "object", "required": [ @@ -437,6 +413,33 @@ } }, "additionalProperties": false + }, + { + "type": "object", + "required": [ + "transfer_status" + ], + "properties": { + "transfer_status": { + "$ref": "#/definitions/Key" + } + }, + "additionalProperties": false + }, + { + "type": "object", + "required": [ + "batch_transfer_status" + ], + "properties": { + "batch_transfer_status": { + "type": "array", + "items": { + "$ref": "#/definitions/Key" + } + } + }, + "additionalProperties": false } ], "definitions": { @@ -963,6 +966,229 @@ } } }, + "batch_transfer_status": { + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "BatchTransferStatusResponse", + "type": "object", + "required": [ + "details" + ], + "properties": { + "details": { + "type": "array", + "items": { + "$ref": "#/definitions/TransferDetails" + } + } + }, + "additionalProperties": false, + "definitions": { + "Binary": { + "description": "Binary is a wrapper around Vec to add base64 de/serialization with serde. It also adds some helper methods to help encode inline.\n\nThis is only needed as serde-json-{core,wasm} has a horrible encoding for Vec", + "type": "string" + }, + "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": "array", + "items": { + "$ref": "#/definitions/Signature" + } + } + }, + "additionalProperties": false + }, + "Key": { + "type": "object", + "required": [ + "emitter_address", + "emitter_chain", + "sequence" + ], + "properties": { + "emitter_address": { + "$ref": "#/definitions/TokenAddress" + }, + "emitter_chain": { + "type": "integer", + "format": "uint16", + "minimum": 0.0 + }, + "sequence": { + "type": "integer", + "format": "uint64", + "minimum": 0.0 + } + }, + "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": "array", + "items": { + "type": "integer", + "format": "uint8", + "minimum": 0.0 + }, + "maxItems": 32, + "minItems": 32 + }, + "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 + }, + "Signature": { + "description": "Signatures are typical ECDSA signatures prefixed with a Guardian position. These have the following byte layout: ```markdown 0 .. 64: Signature (ECDSA) 64 .. 65: Recovery ID (ECDSA) ```", + "type": "object", + "required": [ + "index", + "signature" + ], + "properties": { + "index": { + "type": "integer", + "format": "uint8", + "minimum": 0.0 + }, + "signature": { + "type": "array", + "items": { + "type": "integer", + "format": "uint8", + "minimum": 0.0 + }, + "maxItems": 65, + "minItems": 65 + } + } + }, + "TokenAddress": { + "type": "string" + }, + "TransferDetails": { + "type": "object", + "required": [ + "key" + ], + "properties": { + "key": { + "$ref": "#/definitions/Key" + }, + "status": { + "anyOf": [ + { + "$ref": "#/definitions/TransferStatus" + }, + { + "type": "null" + } + ] + } + }, + "additionalProperties": false + }, + "TransferStatus": { + "oneOf": [ + { + "type": "object", + "required": [ + "pending" + ], + "properties": { + "pending": { + "type": "array", + "items": { + "$ref": "#/definitions/Data" + } + } + }, + "additionalProperties": false + }, + { + "type": "object", + "required": [ + "committed" + ], + "properties": { + "committed": { + "type": "object", + "required": [ + "data", + "digest" + ], + "properties": { + "data": { + "$ref": "#/definitions/Data" + }, + "digest": { + "$ref": "#/definitions/Binary" + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false + } + ] + } + } + }, "chain_registration": { "$schema": "http://json-schema.org/draft-07/schema#", "title": "ChainRegistrationResponse", @@ -1084,37 +1310,81 @@ } } }, - "pending_transfer": { + "transfer_status": { "$schema": "http://json-schema.org/draft-07/schema#", - "title": "Data", - "type": "object", - "required": [ - "guardian_set_index", - "observation", - "signatures" - ], - "properties": { - "guardian_set_index": { - "type": "integer", - "format": "uint32", - "minimum": 0.0 + "title": "TransferStatus", + "oneOf": [ + { + "type": "object", + "required": [ + "pending" + ], + "properties": { + "pending": { + "type": "array", + "items": { + "$ref": "#/definitions/Data" + } + } + }, + "additionalProperties": false }, - "observation": { - "$ref": "#/definitions/Observation" - }, - "signatures": { - "type": "array", - "items": { - "$ref": "#/definitions/Signature" - } + { + "type": "object", + "required": [ + "committed" + ], + "properties": { + "committed": { + "type": "object", + "required": [ + "data", + "digest" + ], + "properties": { + "data": { + "$ref": "#/definitions/Data" + }, + "digest": { + "$ref": "#/definitions/Binary" + } + }, + "additionalProperties": false + } + }, + "additionalProperties": false } - }, - "additionalProperties": false, + ], "definitions": { "Binary": { "description": "Binary is a wrapper around Vec to add base64 de/serialization with serde. It also adds some helper methods to help encode inline.\n\nThis is only needed as serde-json-{core,wasm} has a horrible encoding for Vec", "type": "string" }, + "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": "array", + "items": { + "$ref": "#/definitions/Signature" + } + } + }, + "additionalProperties": false + }, "Observation": { "type": "object", "required": [ @@ -1199,65 +1469,6 @@ } } }, - "transfer": { - "$schema": "http://json-schema.org/draft-07/schema#", - "title": "TransferResponse", - "type": "object", - "required": [ - "data", - "digest" - ], - "properties": { - "data": { - "$ref": "#/definitions/Data" - }, - "digest": { - "$ref": "#/definitions/Binary" - } - }, - "additionalProperties": false, - "definitions": { - "Binary": { - "description": "Binary is a wrapper around Vec to add base64 de/serialization with serde. It also adds some helper methods to help encode inline.\n\nThis is only needed as serde-json-{core,wasm} has a horrible encoding for Vec", - "type": "string" - }, - "Data": { - "type": "object", - "required": [ - "amount", - "recipient_chain", - "token_address", - "token_chain" - ], - "properties": { - "amount": { - "$ref": "#/definitions/Uint256" - }, - "recipient_chain": { - "type": "integer", - "format": "uint16", - "minimum": 0.0 - }, - "token_address": { - "$ref": "#/definitions/TokenAddress" - }, - "token_chain": { - "type": "integer", - "format": "uint16", - "minimum": 0.0 - } - }, - "additionalProperties": false - }, - "TokenAddress": { - "type": "string" - }, - "Uint256": { - "description": "An implementation of u256 that is using strings for JSON encoding/decoding, such that the full u256 range can be used for clients that convert JSON numbers to floats, like JavaScript and jq.\n\n# Examples\n\nUse `from` to create instances out of primitive uint types or `new` to provide big endian bytes:\n\n``` # use cosmwasm_std::Uint256; let a = Uint256::from(258u128); let b = Uint256::new([ 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 0u8, 1u8, 2u8, ]); assert_eq!(a, b); ```", - "type": "string" - } - } - }, "validate_transfer": { "$schema": "http://json-schema.org/draft-07/schema#", "title": "Empty", diff --git a/cosmwasm/contracts/wormchain-accounting/src/contract.rs b/cosmwasm/contracts/wormchain-accounting/src/contract.rs index 961bb9bb6..26c0c8859 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/contract.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/contract.rs @@ -27,11 +27,11 @@ use crate::{ error::{AnyError, ContractError}, msg::{ AllAccountsResponse, AllModificationsResponse, AllPendingTransfersResponse, - AllTransfersResponse, ChainRegistrationResponse, ExecuteMsg, MigrateMsg, - MissingObservation, MissingObservationsResponse, Observation, QueryMsg, TransferResponse, - Upgrade, + AllTransfersResponse, BatchTransferStatusResponse, ChainRegistrationResponse, ExecuteMsg, + MigrateMsg, MissingObservation, MissingObservationsResponse, Observation, QueryMsg, + TransferDetails, TransferStatus, Upgrade, }, - state::{self, Data, PendingTransfer, CHAIN_REGISTRATIONS, DIGESTS, PENDING_TRANSFERS}, + state::{Data, PendingTransfer, CHAIN_REGISTRATIONS, DIGESTS, PENDING_TRANSFERS}, }; // version info for migration info @@ -470,13 +470,9 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { query_all_accounts(deps, start_after, limit).and_then(|resp| to_binary(&resp)) } - QueryMsg::Transfer(req) => query_transfer(deps, req).and_then(|resp| to_binary(&resp)), QueryMsg::AllTransfers { start_after, limit } => { query_all_transfers(deps, start_after, limit).and_then(|resp| to_binary(&resp)) } - QueryMsg::PendingTransfer(req) => { - query_pending_transfer(deps, req).and_then(|resp| to_binary(&resp)) - } QueryMsg::AllPendingTransfers { start_after, limit } => { query_all_pending_transfers(deps, start_after, limit).and_then(|resp| to_binary(&resp)) } @@ -502,6 +498,12 @@ pub fn query(deps: Deps, _env: Env, msg: QueryMsg) -> StdResult { query_missing_observations(deps, guardian_set, index).and_then(|resp| to_binary(&resp)) } + QueryMsg::TransferStatus(key) => { + query_transfer_status(deps, &key).and_then(|resp| to_binary(&resp)) + } + QueryMsg::BatchTransferStatus(keys) => { + query_batch_transfer_status(deps, keys).and_then(|resp| to_binary(&resp)) + } } } @@ -525,21 +527,6 @@ fn query_all_accounts( } } -fn query_transfer(deps: Deps, key: transfer::Key) -> StdResult { - let digest = DIGESTS.load( - deps.storage, - ( - key.emitter_chain(), - key.emitter_address().to_vec(), - key.sequence(), - ), - )?; - - let data = accounting::query_transfer(deps, key)?; - - Ok(TransferResponse { data, digest }) -} - fn query_all_transfers( deps: Deps, start_after: Option, @@ -596,15 +583,6 @@ fn tinyvec_to_vec(tv: TinyVec) -> Vec { } } -fn query_pending_transfer( - deps: Deps, - key: transfer::Key, -) -> StdResult> { - PENDING_TRANSFERS - .load(deps.storage, key) - .map(tinyvec_to_vec) -} - fn query_all_pending_transfers( deps: Deps, start_after: Option, @@ -684,3 +662,41 @@ fn query_missing_observations( Ok(MissingObservationsResponse { missing }) } + +fn query_transfer_status( + deps: Deps, + key: &transfer::Key, +) -> StdResult { + if let Some(digest) = DIGESTS.may_load( + deps.storage, + ( + key.emitter_chain(), + key.emitter_address().to_vec(), + key.sequence(), + ), + )? { + let data = accounting::query_transfer(deps, key.clone())?; + Ok(TransferStatus::Committed { data, digest }) + } else if let Some(data) = PENDING_TRANSFERS.may_load(deps.storage, key.clone())? { + Ok(TransferStatus::Pending(tinyvec_to_vec(data))) + } else { + Err(StdError::not_found(format!("transfer with key {key}"))) + } +} + +fn query_batch_transfer_status( + deps: Deps, + keys: Vec, +) -> StdResult { + keys.into_iter() + .map(|key| { + let status = match query_transfer_status(deps, &key) { + Ok(s) => Some(s), + Err(e) if matches!(e, StdError::NotFound { .. }) => None, + Err(e) => return Err(e), + }; + Ok(TransferDetails { key, status }) + }) + .collect::>>() + .map(|details| BatchTransferStatusResponse { details }) +} diff --git a/cosmwasm/contracts/wormchain-accounting/src/msg.rs b/cosmwasm/contracts/wormchain-accounting/src/msg.rs index 2354ef4e5..9868b8d13 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/msg.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/msg.rs @@ -123,15 +123,11 @@ pub enum QueryMsg { start_after: Option, limit: Option, }, - #[returns(TransferResponse)] - Transfer(transfer::Key), #[returns(AllTransfersResponse)] AllTransfers { start_after: Option, limit: Option, }, - #[returns(state::Data)] - PendingTransfer(transfer::Key), #[returns(AllPendingTransfersResponse)] AllPendingTransfers { start_after: Option, @@ -150,6 +146,10 @@ pub enum QueryMsg { ChainRegistration { chain: u16 }, #[returns(MissingObservationsResponse)] MissingObservations { guardian_set: u32, index: u8 }, + #[returns(TransferStatus)] + TransferStatus(transfer::Key), + #[returns(BatchTransferStatusResponse)] + BatchTransferStatus(Vec), } #[cw_serde] @@ -178,12 +178,6 @@ pub struct ChainRegistrationResponse { pub address: Binary, } -#[cw_serde] -pub struct TransferResponse { - pub data: transfer::Data, - pub digest: Binary, -} - #[cw_serde] pub struct MissingObservationsResponse { pub missing: Vec, @@ -194,3 +188,26 @@ pub struct MissingObservation { pub chain_id: u16, pub tx_hash: Binary, } + +#[cw_serde] +pub enum TransferStatus { + Pending(Vec), + Committed { + data: transfer::Data, + digest: Binary, + }, +} + +#[cw_serde] +pub struct TransferDetails { + // The key for the transfer. + pub key: transfer::Key, + // The status of the transfer. If `status` is `None`, then there is no transfer associated + // with `key`. + pub status: Option, +} + +#[cw_serde] +pub struct BatchTransferStatusResponse { + pub details: Vec, +} diff --git a/cosmwasm/contracts/wormchain-accounting/tests/helpers/mod.rs b/cosmwasm/contracts/wormchain-accounting/tests/helpers/mod.rs index 9ccc2c152..4b2492a64 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/helpers/mod.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/helpers/mod.rs @@ -1,9 +1,10 @@ #![allow(dead_code)] use accounting::state::{account, transfer, Modification}; +use cosmwasm_schema::cw_serde; use cosmwasm_std::{ testing::{MockApi, MockStorage}, - Addr, Binary, Coin, Empty, StdResult, Uint128, + Addr, Binary, Coin, Empty, StdError, StdResult, Uint128, }; use cw_multi_test::{ App, AppBuilder, AppResponse, BankKeeper, ContractWrapper, Executor, WasmKeeper, @@ -12,8 +13,8 @@ use serde::Serialize; use wormchain_accounting::{ msg::{ AllAccountsResponse, AllModificationsResponse, AllPendingTransfersResponse, - AllTransfersResponse, ChainRegistrationResponse, ExecuteMsg, MissingObservationsResponse, - QueryMsg, TransferResponse, + AllTransfersResponse, BatchTransferStatusResponse, ChainRegistrationResponse, ExecuteMsg, + MissingObservationsResponse, QueryMsg, TransferStatus, }, state, }; @@ -24,6 +25,12 @@ use wormhole::{ }; use wormhole_bindings::{fake, WormholeQuery}; +#[cw_serde] +pub struct TransferResponse { + pub data: transfer::Data, + pub digest: Binary, +} + pub struct Contract { addr: Addr, app: FakeApp, @@ -121,10 +128,29 @@ impl Contract { .query_wasm_smart(self.addr(), &QueryMsg::AllAccounts { start_after, limit }) } - pub fn query_transfer(&self, key: transfer::Key) -> StdResult { + pub fn query_transfer_status(&self, key: transfer::Key) -> StdResult { self.app .wrap() - .query_wasm_smart(self.addr(), &QueryMsg::Transfer(key)) + .query_wasm_smart(self.addr(), &QueryMsg::TransferStatus(key)) + } + + pub fn query_batch_transfer_status( + &self, + keys: Vec, + ) -> StdResult { + self.app + .wrap() + .query_wasm_smart(self.addr(), &QueryMsg::BatchTransferStatus(keys)) + } + + pub fn query_transfer(&self, key: transfer::Key) -> StdResult { + self.query_transfer_status(key.clone()).and_then(|status| { + if let TransferStatus::Committed { data, digest } = status { + Ok(TransferResponse { data, digest }) + } else { + Err(StdError::not_found(format!("transfer for key {key}"))) + } + }) } pub fn query_all_transfers( @@ -138,9 +164,15 @@ impl Contract { } pub fn query_pending_transfer(&self, key: transfer::Key) -> StdResult> { - self.app - .wrap() - .query_wasm_smart(self.addr(), &QueryMsg::PendingTransfer(key)) + self.query_transfer_status(key.clone()).and_then(|status| { + if let TransferStatus::Pending(state) = status { + Ok(state) + } else { + Err(StdError::not_found(format!( + "pending transfer for key {key}" + ))) + } + }) } pub fn query_all_pending_transfers( diff --git a/cosmwasm/contracts/wormchain-accounting/tests/query.rs b/cosmwasm/contracts/wormchain-accounting/tests/query.rs index 18ecbd917..8c18a4eb1 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/query.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/query.rs @@ -8,6 +8,7 @@ use accounting::state::{ }; use cosmwasm_std::{to_binary, Uint256}; use helpers::*; +use wormchain_accounting::msg::TransferStatus; use wormhole::{token::Message, vaa::Body, Address, Amount}; use wormhole_bindings::fake; @@ -272,6 +273,24 @@ fn all_transfer_data() { } } +#[test] +fn batch_transfer_status() { + let count = 3; + let (wh, mut contract) = proper_instantiate(); + let transfers = create_transfers(&wh, &mut contract, count); + + let keys = transfers.iter().map(|t| &t.key).cloned().collect(); + let resp = contract.query_batch_transfer_status(keys).unwrap(); + + for (tx, details) in transfers.into_iter().zip(resp.details) { + assert_eq!(tx.key, details.key); + match details.status { + Some(TransferStatus::Committed { data, .. }) => assert_eq!(tx.data, data), + s => panic!("unexpected transfer status: {s:?}"), + } + } +} + #[test] fn all_transfer_data_sub_range() { let count = 5;