diff --git a/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json b/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json index 4be6f5c40..b3cc0aa5c 100644 --- a/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json +++ b/cosmwasm/contracts/wormchain-accounting/schema/wormchain-accounting.json @@ -705,10 +705,9 @@ "$ref": "#/definitions/Observation" }, "signatures": { - "type": "array", - "items": { - "$ref": "#/definitions/Signature" - } + "type": "integer", + "format": "uint128", + "minimum": 0.0 } }, "additionalProperties": false @@ -806,31 +805,6 @@ }, "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" } @@ -997,10 +971,9 @@ "$ref": "#/definitions/Observation" }, "signatures": { - "type": "array", - "items": { - "$ref": "#/definitions/Signature" - } + "type": "integer", + "format": "uint128", + "minimum": 0.0 } }, "additionalProperties": false @@ -1079,31 +1052,6 @@ }, "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" }, @@ -1363,10 +1311,9 @@ "$ref": "#/definitions/Observation" }, "signatures": { - "type": "array", - "items": { - "$ref": "#/definitions/Signature" - } + "type": "integer", + "format": "uint128", + "minimum": 0.0 } }, "additionalProperties": false @@ -1420,31 +1367,6 @@ } }, "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 - } - } } } }, diff --git a/cosmwasm/contracts/wormchain-accounting/src/contract.rs b/cosmwasm/contracts/wormchain-accounting/src/contract.rs index ad5a42783..e4302c265 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/contract.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/contract.rs @@ -109,11 +109,6 @@ fn submit_observations( let quorum = deps .querier .query::(&WormholeQuery::CalculateQuorum { guardian_set_index }.into()) - .and_then(|q| { - usize::try_from(q).map_err(|_| StdError::ConversionOverflow { - source: ConversionOverflowError::new("u32", "usize", q.to_string()), - }) - }) .context("failed to calculate quorum")?; let observations: Vec = @@ -159,7 +154,7 @@ fn handle_observation( mut deps: DepsMut, o: Observation, guardian_set_index: u32, - quorum: usize, + quorum: u32, sig: Signature, ) -> anyhow::Result<(ObservationStatus, Option)> { let digest_key = DIGESTS.key((o.emitter_chain, o.emitter_address.to_vec(), o.sequence)); @@ -194,9 +189,9 @@ fn handle_observation( } }; - data.add_signature(sig)?; + data.add_signature(sig.index); - if data.signatures().len() < quorum { + if data.num_signatures() < quorum { // Still need more signatures so just save the pending transfer data and exit. key.save(deps.storage, &pending) .context("failed to save pending transfers")?; diff --git a/cosmwasm/contracts/wormchain-accounting/src/state.rs b/cosmwasm/contracts/wormchain-accounting/src/state.rs index bb05d2d4a..f1b4563eb 100644 --- a/cosmwasm/contracts/wormchain-accounting/src/state.rs +++ b/cosmwasm/contracts/wormchain-accounting/src/state.rs @@ -2,9 +2,7 @@ use accounting::state::transfer; use cosmwasm_schema::cw_serde; use cosmwasm_std::Binary; use cw_storage_plus::Map; -use thiserror::Error; use tinyvec::TinyVec; -use wormhole::vaa::Signature; use crate::msg::Observation; @@ -18,18 +16,12 @@ pub struct PendingTransfer { pub data: Vec, } -#[derive(Error, Debug)] -#[error("cannot submit duplicate signatures for the same observation")] -pub struct DuplicateSignatureError; - #[cw_serde] #[derive(Default)] pub struct Data { observation: Observation, - guardian_set_index: u32, - - signatures: Vec, + signatures: u128, } impl Data { @@ -37,7 +29,7 @@ impl Data { Self { observation, guardian_set_index, - signatures: Vec::new(), + signatures: 0, } } @@ -49,29 +41,69 @@ impl Data { self.guardian_set_index } - pub fn signatures(&self) -> &[Signature] { - &self.signatures + pub fn signatures(&self) -> u128 { + self.signatures + } + + /// Returns the number of signatures for this `Data`. + pub fn num_signatures(&self) -> u32 { + self.signatures.count_ones() } /// Returns true if there is a signature associated with `index` in this `Data`. pub fn has_signature(&self, index: u8) -> bool { - self.signatures - .binary_search_by_key(&index, |s| s.index) - .is_ok() + assert!(index < 128); + self.signatures & (1u128 << index) != 0 } - /// Adds `sig` to the list of signatures for this transfer data. Returns true if `sig` - /// was successfully added or false if `sig` was already in the signature list. - pub fn add_signature(&mut self, sig: Signature) -> Result<(), DuplicateSignatureError> { - match self - .signatures - .binary_search_by_key(&sig.index, |s| s.index) - { - Ok(_) => Err(DuplicateSignatureError), - Err(idx) => { - self.signatures.insert(idx, sig); - Ok(()) - } + /// Adds `sig` to the list of signatures for this `Data`. + pub fn add_signature(&mut self, index: u8) { + assert!(index < 128); + self.signatures |= 1u128 << index; + } +} + +#[cfg(test)] +mod test { + use super::*; + + use std::panic::catch_unwind; + + #[test] + fn add_signatures() { + let mut data = Data::default(); + for i in 0..128 { + assert!(!data.has_signature(i)); + data.add_signature(i); + assert!(data.has_signature(i)); + assert_eq!(u32::from(i + 1), data.num_signatures()); + } + } + + #[test] + fn add_signatures_rev() { + let mut data = Data::default(); + for i in (0..128).rev() { + assert!(!data.has_signature(i)); + data.add_signature(i); + assert!(data.has_signature(i)); + assert_eq!(u32::from(128 - i), data.num_signatures()); + } + } + + #[test] + fn has_out_of_bounds_signature() { + for i in 128..=u8::MAX { + catch_unwind(|| Data::default().has_signature(i)) + .expect_err("successfully checked for out-of-bounds signature"); + } + } + + #[test] + fn add_out_of_bounds_signature() { + for i in 128..=u8::MAX { + catch_unwind(|| Data::default().add_signature(i)) + .expect_err("successfully added out-of-bounds signature"); } } } diff --git a/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs b/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs index 417dbad9a..99f26e09f 100644 --- a/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs +++ b/cosmwasm/contracts/wormchain-accounting/tests/submit_observations.rs @@ -166,6 +166,8 @@ fn duplicates() { for (i, s) in signatures.into_iter().enumerate() { contract.submit_observations(obs.clone(), index, s).unwrap(); + // Submitting a duplicate signature is not an error for pending transfers. Submitting any + // signature for a committed transfer is not an error as long as the digests match. let resp = contract.submit_observations(obs.clone(), index, s).unwrap(); let status = from_binary::>(&resp.data.unwrap()) .unwrap() @@ -176,18 +178,9 @@ fn duplicates() { // Resubmitting the same signature without quorum will return an error. for o in &observations { let key = transfer::Key::new(o.emitter_chain, o.emitter_address.into(), o.sequence); - if let ObservationStatus::Error(ref err) = status[&key] { - assert!(err.contains("duplicate signatures")); - } else { - panic!( - "unexpected status for duplicate signature: {:?}", - status[&key] - ); - } + assert!(matches!(status[&key], ObservationStatus::Pending)); } } else { - // Submitting a signature for a committed transfer is not an error as long as the - // digests match. for o in &observations { let key = transfer::Key::new(o.emitter_chain, o.emitter_address.into(), o.sequence); assert!(matches!(status[&key], ObservationStatus::Committed));