From d19fc98091719477bed5702651bb0196e509b697 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Mon, 16 Jan 2023 13:21:17 +0900 Subject: [PATCH] sdk/rust: Properly support tokenbridge payload3 messages Add the payload as an explicit field to the `TransferWithPayload` enum variant. This is a generic parameter that defaults to `Box` for maximum flexibility (and to avoid leaking lifetimes higher up the stack) but users are encouraged to replace this default type parameter with an explicit `&RawMessage` in places where the serde_wormhole data format is used. The main benefit of this change is that the payload is now included as part of the actual message and no longer requires callers to awkwardly append it after serialization. This is especially useful in human- readable formats like JSON (see the `transfer_with_payload` test in token.rs for an example of this simplification). The main downside is that this now requires explicit type annotations when using the non-payload3 variants so that the compiler will pick up the default generic parameter. This is a relatively minor inconvenience and the benefit appears to be worth the cost. There should be no functional change. --- .../wormchain-accounting/src/contract.rs | 6 +- .../contracts/wormchain-accounting/src/msg.rs | 8 +- .../tests/missing_observations.rs | 4 +- .../wormchain-accounting/tests/query.rs | 2 +- .../wormchain-accounting/tests/submit_vaas.rs | 16 ++-- sdk/rust/core/src/token.rs | 96 ++++++++++++------- sdk/rust/core/src/vaa.rs | 7 -- 7 files changed, 78 insertions(+), 61 deletions(-) diff --git a/cosmwasm/contracts/wormchain-accounting/src/contract.rs b/cosmwasm/contracts/wormchain-accounting/src/contract.rs index 330dd84a6..df732ab3a 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/contract.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/contract.rs @@ -184,7 +184,7 @@ fn handle_observation( return Ok(None); } - let (msg, _) = serde_wormhole::from_slice::<(Message, &RawMessage)>(&o.payload) + let msg = serde_wormhole::from_slice::>(&o.payload) .context("failed to parse observation payload")?; let tx_data = match msg { Message::Transfer { @@ -370,7 +370,7 @@ fn handle_vaa(mut deps: DepsMut, vaa: Binary) -> anyhow::Result(body.payload) + let msg = serde_wormhole::from_slice(body.payload) .context("failed to parse tokenbridge message")?; handle_tokenbridge_vaa(deps.branch(), body.with_payload(msg))? }; @@ -413,7 +413,7 @@ fn handle_governance_vaa( fn handle_tokenbridge_vaa( mut deps: DepsMut, - body: Body, + body: Body>, ) -> anyhow::Result { let data = match body.payload { Message::Transfer { diff --git a/cosmwasm/contracts/wormchain-accounting/src/msg.rs b/cosmwasm/contracts/wormchain-accounting/src/msg.rs index 2274555c4..04e7b85ae 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/msg.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/msg.rs @@ -1,6 +1,7 @@ use accounting::state::{account, transfer, Account, Modification, Transfer}; use cosmwasm_schema::{cw_serde, QueryResponses}; use cosmwasm_std::Binary; +use serde_wormhole::RawMessage; use wormhole::{ vaa::{Body, Signature}, Address, @@ -41,9 +42,6 @@ pub struct Observation { impl Observation { // Calculate a digest of the observation that can be used for de-duplication. pub fn digest(&self) -> anyhow::Result { - // We don't know the actual type of `self.payload` so we create a body with a 0-sized - // payload and then just append it when calculating the digest. - let body = Body { timestamp: self.timestamp, nonce: self.nonce, @@ -51,10 +49,10 @@ impl Observation { emitter_address: Address(self.emitter_address), sequence: self.sequence, consistency_level: self.consistency_level, - payload: (), + payload: RawMessage::new(&self.payload), }; - let digest = body.digest_with_payload(&self.payload)?; + let digest = body.digest()?; Ok(digest.secp256k_hash.to_vec().into()) } diff --git a/cosmwasm/contracts/wormchain-accounting/tests/missing_observations.rs b/cosmwasm/contracts/wormchain-accounting/tests/missing_observations.rs index bf36764c3..6be2be8bb 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/missing_observations.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/missing_observations.rs @@ -7,7 +7,7 @@ use wormchain_accounting::msg::Observation; use wormhole::{token::Message, Address, Amount, Chain}; fn create_observation() -> Observation { - let msg = Message::Transfer { + let msg: Message = Message::Transfer { amount: Amount(Uint256::from(500u128).to_be_bytes()), token_address: Address([0x02; 32]), token_chain: Chain::Ethereum, @@ -106,7 +106,7 @@ fn different_observations() { } // Create a new observation with a different tx hash and payload. - let msg = Message::Transfer { + let msg: Message = Message::Transfer { amount: Amount(Uint256::from(900u128).to_be_bytes()), token_address: Address([0x02; 32]), token_chain: Chain::Ethereum, diff --git a/cosmwasm/contracts/wormchain-accounting/tests/query.rs b/cosmwasm/contracts/wormchain-accounting/tests/query.rs index 8c18a4eb1..d0a8e381e 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/query.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/query.rs @@ -53,7 +53,7 @@ fn create_transfers( let recipient_chain = emitter_chain + 1; let amount = Uint256::from(i as u128); - let body = Body { + let body: Body = Body { timestamp: i as u32, nonce: i as u32, emitter_chain: emitter_chain.into(), diff --git a/cosmwasm/contracts/wormchain-accounting/tests/submit_vaas.rs b/cosmwasm/contracts/wormchain-accounting/tests/submit_vaas.rs index b00ef88a3..a3b95b94d 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/submit_vaas.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/submit_vaas.rs @@ -3,6 +3,7 @@ mod helpers; use accounting::state::{transfer, TokenAddress}; use cosmwasm_std::{to_binary, Binary, Event, Uint256}; use helpers::*; +use serde_wormhole::RawMessage; use wormchain_accounting::msg::Observation; use wormhole::{ token::Message, @@ -43,7 +44,7 @@ fn create_vaa_body(i: usize) -> Body { } } -fn transfer_data_from_token_message(msg: Message) -> transfer::Data { +fn transfer_data_from_token_message

(msg: Message

) -> transfer::Data { match msg { Message::Transfer { amount, @@ -172,7 +173,7 @@ fn bad_signature() { #[test] fn non_transfer_message() { let (wh, mut contract) = proper_instantiate(); - let body = Body { + let body: Body = Body { timestamp: 2, nonce: 2, emitter_chain: Chain::Ethereum, @@ -201,6 +202,7 @@ fn non_transfer_message() { #[test] fn transfer_with_payload() { let (wh, mut contract) = proper_instantiate(); + let payload = [0x88; 17]; let body = Body { timestamp: 2, nonce: 2, @@ -215,12 +217,11 @@ fn transfer_with_payload() { recipient: Address([2u8; 32]), recipient_chain: Chain::Bsc, sender_address: Address([0u8; 32]), + payload: RawMessage::new(&payload[..]), }, }; - let payload = [0x88; 17]; - let mut data = serde_wormhole::to_vec(&body).unwrap(); - data.extend_from_slice(&payload); + let data = serde_wormhole::to_vec(&body).unwrap(); let signatures = wh.sign(&data); let header = Header { @@ -231,8 +232,7 @@ fn transfer_with_payload() { let v = Vaa::from((header, body)); - let mut data = serde_wormhole::to_vec(&v).unwrap(); - data.extend_from_slice(&payload); + let data = serde_wormhole::to_vec(&v).unwrap(); let resp = contract.submit_vaas(vec![data.into()]).unwrap(); let key = transfer::Key::new( @@ -241,7 +241,7 @@ fn transfer_with_payload() { v.sequence, ); let (_, body) = v.into(); - let digest = body.digest_with_payload(&payload).unwrap().secp256k_hash; + let digest = body.digest().unwrap().secp256k_hash; let data = transfer_data_from_token_message(body.payload); let tx = contract.query_transfer(key.clone()).unwrap(); assert_eq!(data, tx.data); diff --git a/sdk/rust/core/src/token.rs b/sdk/rust/core/src/token.rs index 7ebad189b..4a5621adf 100644 --- a/sdk/rust/core/src/token.rs +++ b/sdk/rust/core/src/token.rs @@ -7,12 +7,21 @@ use bstr::BString; use serde::{Deserialize, Serialize}; +use serde_wormhole::RawMessage; use crate::{Address, Amount, Chain}; /// Represents a non-governance action targeted at the token bridge. +/// +/// The generic parameter `P` indicates the type of the payload for the `TransferWithPayload` +/// variant. This defaults to `Box` as that provides the most flexibility when +/// deserializing the payload and avoids leaking lifetime parameters higher up the stack. However, +/// users who are serializing to or deserializing from the serde_wormhole data format may want to +/// use a `&RawMessage` to avoid unnecessary memory allocations. When the type of the payload is +/// known and is serialized in the same data format as the rest of the message, that type can be +/// used as the generic parameter to deserialize the message and the payload together. #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub enum Message { +pub enum Message

> { /// The Transfer message contains specifics detailing a token lock up on a sending chain. Chains /// that are attempting to initiate a transfer must lock up tokens in some manner, such as in a /// custody account or via burning, before emitting this message. @@ -58,8 +67,7 @@ pub enum Message { name: BString, }, - /// Similar to `Transfer` but also includes an arbitrary payload that is appended to the end of - /// the message. + /// Similar to `Transfer` but also includes an arbitrary payload. /// /// # Examples /// @@ -96,9 +104,17 @@ pub enum Message { /// use serde_wormhole::RawMessage; /// use wormhole::{token::Message, Vaa}; /// - /// let (msg, payload) = serde_wormhole::from_slice::<(Vaa, &RawMessage)>(&data)?; - /// assert!(matches!(msg.payload, Message::TransferWithPayload { .. })); - /// assert_eq!(&data[256..], payload.get()); + /// let msg = serde_wormhole::from_slice::>>(&data)?; + /// match msg.payload { + /// Message::TransferWithPayload { payload, .. } => { + /// // Handle the payload. + /// # assert_eq!(&data[256..], payload.get()) + /// } + /// _ => { + /// // Handle other message types. + /// # panic!("unexpected message type") + /// } + /// } /// # /// # Ok(()) /// # } @@ -110,9 +126,13 @@ pub enum Message { /// /// ``` /// # fn example() -> anyhow::Result<()> { + /// # use serde_wormhole::RawMessage; /// # use wormhole::{Address, Amount, Chain, vaa::Signature, GOVERNANCE_EMITTER}; - /// use anyhow::anyhow; - /// use wormhole::{token::Message, Vaa}; + /// # let tx_payload = [ + /// # 0x93, 0xd7, 0xc0, 0x9e, 0xe8, 0x87, 0xae, 0x16, 0xbf, 0xfa, 0x5e, 0x70, 0xea, 0x36, 0xa2, + /// # 0x82, 0x37, 0x1d, 0x46, 0x81, 0x94, 0x10, 0x34, 0xb1, 0xad, 0x0f, 0x4b, 0xc9, + /// # ]; + /// # /// # let vaa = Vaa { /// # version: 1, /// # guardian_set_index: 0, @@ -130,25 +150,27 @@ pub enum Message { /// # recipient: Address([0x19; 32]), /// # recipient_chain: Chain::Osmosis, /// # sender_address: Address([0xfe; 32]), + /// # payload: >::from(tx_payload.to_vec()), /// # }, /// # }; /// # - /// # let payload = [ - /// # 0x93, 0xd7, 0xc0, 0x9e, 0xe8, 0x87, 0xae, 0x16, 0xbf, 0xfa, 0x5e, 0x70, 0xea, 0x36, 0xa2, - /// # 0x82, 0x37, 0x1d, 0x46, 0x81, 0x94, 0x10, 0x34, 0xb1, 0xad, 0x0f, 0x4b, 0xc9, - /// # ]; - /// # - /// # let mut data = serde_json::to_vec(&vaa)?; - /// # data.extend_from_slice(&payload); + /// # let data = serde_json::to_vec(&vaa)?; + /// use anyhow::anyhow; + /// use wormhole::{token::Message, Vaa}; /// - /// let mut stream = serde_json::Deserializer::from_slice(&data).into_iter(); - /// let msg: Vaa = stream - /// .next() - /// .ok_or_else(|| anyhow!("missing token message"))??; - /// assert!(matches!(msg.payload, Message::TransferWithPayload { .. })); - /// assert_eq!(&data[stream.byte_offset()..], payload); - /// # + /// let msg = serde_json::from_slice::>(&data)?; /// # assert_eq!(vaa, msg); + /// match msg.payload { + /// Message::TransferWithPayload { payload, .. } => { + /// // Handle the payload. + /// # assert_eq!(&tx_payload[..], payload.get()) + /// } + /// _ => { + /// // Handle other message types. + /// # panic!("unexpected message type") + /// } + /// } + /// # /// # Ok(()) /// # } /// # @@ -173,7 +195,9 @@ pub enum Message { /// The identity of the sender sending the payload. sender_address: Address, - // The actual payload is appended to the end of the message. + + /// The payload to be included with the transfer. + payload: P, }, } @@ -479,7 +503,9 @@ mod test { 0x3b, 0xd0, 0x0a, 0x2f, 0xdd, 0x02, 0xe8, 0xef, 0xfc, 0x7a, 0xfe, 0x3d, 0xd6, 0x73, 0x2c, 0x5d, 0xa0, 0x6b, 0x08, 0x98, 0x6c, ]; - let msg = Message::Transfer { + // Need to explicity annotate the type here so that the compiler will use the default type + // parameter. + let msg: Message = Message::Transfer { amount: Amount([ 0xfa, 0x22, 0x8b, 0x3d, 0xf3, 0x59, 0x21, 0xa1, 0xc9, 0x39, 0xad, 0x9c, 0x54, 0xe1, 0x2f, 0x87, 0xc6, 0x27, 0x25, 0xd1, 0xaf, 0x7c, 0x7b, 0x6a, 0xea, 0xbf, 0x48, 0x14, @@ -524,7 +550,9 @@ mod test { 0x65, 0x6e, ]; - let msg = Message::AssetMeta { + // Need to explicity annotate the type here so that the compiler will use the default type + // parameter. + let msg: Message = Message::AssetMeta { token_address: Address([ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, @@ -562,6 +590,9 @@ mod test { 0xa2, 0x82, 0x37, 0x1d, 0x46, 0x81, 0x94, 0x10, 0x34, 0xb1, 0xad, 0x0f, 0x4b, 0xc9, 0x17, 0x1e, 0x91, 0x25, 0x11, ]; + + // No need to annotate the type here as the compiler can infer the generic parameter via the + // `TransferWithPayload` variant. let msg = Message::TransferWithPayload { amount: Amount([ 0x99, 0x30, 0x29, 0x01, 0x67, 0xf3, 0x48, 0x6a, 0xf6, 0x4c, 0xdd, 0x07, 0x64, 0xa1, @@ -585,19 +616,14 @@ mod test { 0x41, 0xcf, 0x74, 0x24, 0xff, 0xa4, 0x02, 0x35, 0xbe, 0xb1, 0x7c, 0x47, 0x16, 0xba, 0xbc, 0xaa, 0xbe, 0x99, ]), + payload: >::from(payload[133..].to_vec()), }; - assert_eq!(&payload[..133], &serde_wormhole::to_vec(&msg).unwrap()); - let (actual, data) = serde_wormhole::from_slice::<(_, &RawMessage)>(&payload).unwrap(); - assert_eq!(msg, actual); - assert_eq!(&payload[133..], data.get()); + assert_eq!(&payload[..], &serde_wormhole::to_vec(&msg).unwrap()); + assert_eq!(msg, serde_wormhole::from_slice(&payload).unwrap()); - let mut encoded = serde_json::to_vec(&msg).unwrap(); - encoded.extend_from_slice(&payload[133..]); - - let mut stream = serde_json::Deserializer::from_slice(&encoded).into_iter(); - assert_eq!(msg, stream.next().unwrap().unwrap()); - assert_eq!(payload[133..], encoded[stream.byte_offset()..]); + let encoded = serde_json::to_vec(&msg).unwrap(); + assert_eq!(msg, serde_json::from_slice(&encoded).unwrap()); } #[test] diff --git a/sdk/rust/core/src/vaa.rs b/sdk/rust/core/src/vaa.rs index 2ed8f28fc..ea9c9f530 100644 --- a/sdk/rust/core/src/vaa.rs +++ b/sdk/rust/core/src/vaa.rs @@ -246,17 +246,10 @@ impl Body

{ /// and hashing the result using on-chain primitives. #[inline] pub fn digest(&self) -> anyhow::Result { - self.digest_with_payload(&[]) - } - - /// Like `digest` but allows specifying an additional payload to include in the body hash. - pub fn digest_with_payload(&self, payload: &[u8]) -> anyhow::Result { // The `body` of the VAA is hashed to produce a `digest` of the VAA. let hash: [u8; 32] = { let mut h = sha3::Keccak256::default(); serde_wormhole::to_writer(&mut h, self).context("failed to serialize body")?; - h.write_all(payload) - .context("failed to hash extra payload")?; h.finalize().into() };