From 19d6eaf9c51b4cce1a5df566a03060c9a556562a Mon Sep 17 00:00:00 2001 From: elizabeth Date: Fri, 1 Nov 2024 12:37:56 -0400 Subject: [PATCH 1/5] use latest for queries packet/ack/receipt and return proof height --- crates/relayer/src/chain/astria/endpoint.rs | 59 +++++++++++--- crates/relayer/src/chain/cosmos.rs | 18 ++--- crates/relayer/src/chain/endpoint.rs | 87 ++++++++++++--------- crates/relayer/src/chain/handle.rs | 12 +-- crates/relayer/src/chain/handle/base.rs | 6 +- crates/relayer/src/chain/handle/cache.rs | 6 +- crates/relayer/src/chain/handle/counting.rs | 6 +- crates/relayer/src/chain/runtime.rs | 6 +- crates/relayer/src/transfer.rs | 20 ++--- 9 files changed, 130 insertions(+), 90 deletions(-) diff --git a/crates/relayer/src/chain/astria/endpoint.rs b/crates/relayer/src/chain/astria/endpoint.rs index 1bccf16189..841dae794b 100644 --- a/crates/relayer/src/chain/astria/endpoint.rs +++ b/crates/relayer/src/chain/astria/endpoint.rs @@ -26,6 +26,7 @@ use ibc_relayer_types::{ ics02_client::{ client_type::ClientType, events::UpdateClient, + height::Height, }, ics03_connection::connection::{ ConnectionEnd, @@ -1107,7 +1108,7 @@ impl ChainEndpoint for AstriaEndpoint { &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { let mut client = self.ibc_channel_grpc_client.clone(); let req = ibc_proto::ibc::core::channel::v1::QueryPacketCommitmentRequest { port_id: request.port_id.to_string(), @@ -1132,10 +1133,20 @@ impl ChainEndpoint for AstriaEndpoint { .into_inner(); match include_proof { - IncludeProof::Yes => Ok(( - response.commitment, - Some(decode_merkle_proof(response.proof)?), - )), + IncludeProof::Yes => { + let proof = decode_merkle_proof(response.proof)?; + let height = response + .proof_height + .ok_or(Error::grpc_response_param("proof_height".to_string()))?; + Ok(( + response.commitment, + Some(( + proof, + Height::new(height.revision_number, height.revision_height) + .map_err(|e| Error::other(Box::new(e)))?, + )), + )) + } IncludeProof::No => Ok((response.commitment, None)), } } @@ -1176,7 +1187,7 @@ impl ChainEndpoint for AstriaEndpoint { &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { let mut client = self.ibc_channel_grpc_client.clone(); let req = ibc_proto::ibc::core::channel::v1::QueryPacketReceiptRequest { port_id: request.port_id.to_string(), @@ -1200,14 +1211,26 @@ impl ChainEndpoint for AstriaEndpoint { .map_err(|e| Error::grpc_status(e, "query_packet_receipt".to_owned()))? .into_inner(); - // TODO: is this right? let value = match response.received { true => vec![1], false => vec![0], }; match include_proof { - IncludeProof::Yes => Ok((value, Some(decode_merkle_proof(response.proof)?))), + IncludeProof::Yes => { + let proof = decode_merkle_proof(response.proof)?; + let height = response + .proof_height + .ok_or(Error::grpc_response_param("proof_height".to_string()))?; + Ok(( + value, + Some(( + proof, + Height::new(height.revision_number, height.revision_height) + .map_err(|e| Error::other(Box::new(e)))?, + )), + )) + } IncludeProof::No => Ok((value, None)), } } @@ -1247,7 +1270,7 @@ impl ChainEndpoint for AstriaEndpoint { &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { let mut client = self.ibc_channel_grpc_client.clone(); let req = ibc_proto::ibc::core::channel::v1::QueryPacketAcknowledgementRequest { port_id: request.port_id.to_string(), @@ -1272,10 +1295,20 @@ impl ChainEndpoint for AstriaEndpoint { .into_inner(); match include_proof { - IncludeProof::Yes => Ok(( - response.acknowledgement, - Some(decode_merkle_proof(response.proof)?), - )), + IncludeProof::Yes => { + let proof = decode_merkle_proof(response.proof)?; + let height = response + .proof_height + .ok_or(Error::grpc_response_param("proof_height".to_string()))?; + Ok(( + response.acknowledgement, + Some(( + proof, + Height::new(height.revision_number, height.revision_height) + .map_err(|e| Error::other(Box::new(e)))?, + )), + )) + } IncludeProof::No => Ok((response.acknowledgement, None)), } } diff --git a/crates/relayer/src/chain/cosmos.rs b/crates/relayer/src/chain/cosmos.rs index e89224f7b5..b48c8cb523 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -1832,7 +1832,7 @@ impl ChainEndpoint for CosmosSdkChain { &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, ibc_relayer_types::Height)>), Error> { let res = self.query( CommitmentsPath { port_id: request.port_id, @@ -1846,8 +1846,8 @@ impl ChainEndpoint for CosmosSdkChain { match include_proof { IncludeProof::Yes => { let proof = res.proof.ok_or_else(Error::empty_response_proof)?; - - Ok((res.value, Some(proof))) + let height = ibc_relayer_types::Height::from_tm(res.height, &self.config.id); + Ok((res.value, Some((proof, height)))) } IncludeProof::No => Ok((res.value, None)), } @@ -1904,7 +1904,7 @@ impl ChainEndpoint for CosmosSdkChain { &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, ibc_relayer_types::Height)>), Error> { let res = self.query( ReceiptsPath { port_id: request.port_id, @@ -1918,8 +1918,8 @@ impl ChainEndpoint for CosmosSdkChain { match include_proof { IncludeProof::Yes => { let proof = res.proof.ok_or_else(Error::empty_response_proof)?; - - Ok((res.value, Some(proof))) + let height = ibc_relayer_types::Height::from_tm(res.height, &self.config.id); + Ok((res.value, Some((proof, height)))) } IncludeProof::No => Ok((res.value, None)), } @@ -1969,7 +1969,7 @@ impl ChainEndpoint for CosmosSdkChain { &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, ibc_relayer_types::Height)>), Error> { let res = self.query( AcksPath { port_id: request.port_id, @@ -1983,8 +1983,8 @@ impl ChainEndpoint for CosmosSdkChain { match include_proof { IncludeProof::Yes => { let proof = res.proof.ok_or_else(Error::empty_response_proof)?; - - Ok((res.value, Some(proof))) + let height = ibc_relayer_types::Height::from_tm(res.height, &self.config.id); + Ok((res.value, Some((proof, height)))) } IncludeProof::No => Ok((res.value, None)), } diff --git a/crates/relayer/src/chain/endpoint.rs b/crates/relayer/src/chain/endpoint.rs index 7a2a83498a..8d0a7172a4 100644 --- a/crates/relayer/src/chain/endpoint.rs +++ b/crates/relayer/src/chain/endpoint.rs @@ -319,7 +319,7 @@ pub trait ChainEndpoint: Sized { &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error>; + ) -> Result<(Vec, Option<(MerkleProof, ICSHeight)>), Error>; /// Performs a query to retrieve all the packet commitments hashes /// associated with a channel. Returns the corresponding packet sequence @@ -335,7 +335,7 @@ pub trait ChainEndpoint: Sized { &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error>; + ) -> Result<(Vec, Option<(MerkleProof, ICSHeight)>), Error>; /// Performs a query about which IBC packets in the specified list has not /// been received. Returns the sequence numbers of the packets that were not @@ -356,7 +356,7 @@ pub trait ChainEndpoint: Sized { &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error>; + ) -> Result<(Vec, Option<(MerkleProof, ICSHeight)>), Error>; /// Performs a query to retrieve all the packet acknowledgements associated /// with a channel. Returns the corresponding packet sequence numbers and @@ -576,67 +576,88 @@ pub trait ChainEndpoint: Sized { port_id: PortId, channel_id: ChannelId, sequence: Sequence, - height: ICSHeight, + query_height: ICSHeight, ) -> Result { - let (maybe_packet_proof, channel_proof) = match packet_type { + let (height, packet_proof, channel_proof) = match packet_type { PacketMsgType::Recv => { - let (_, maybe_packet_proof) = self.query_packet_commitment( + let (_, proof_and_height) = self.query_packet_commitment( QueryPacketCommitmentRequest { port_id, channel_id, sequence, - height: QueryHeight::Specific(height), + height: QueryHeight::Latest, }, IncludeProof::Yes, )?; - (maybe_packet_proof, None) + let proof_and_height = + proof_and_height.ok_or_else(Error::queried_proof_not_found)?; + (proof_and_height.1, proof_and_height.0, None) } PacketMsgType::Ack => { - let (_, maybe_packet_proof) = self.query_packet_acknowledgement( + let (_, proof_and_height) = self.query_packet_acknowledgement( QueryPacketAcknowledgementRequest { port_id, channel_id, sequence, - height: QueryHeight::Specific(height), + height: QueryHeight::Latest, }, IncludeProof::Yes, )?; - (maybe_packet_proof, None) + let proof_and_height = + proof_and_height.ok_or_else(Error::queried_proof_not_found)?; + (proof_and_height.1, proof_and_height.0, None) } PacketMsgType::TimeoutUnordered => { - let (_, maybe_packet_proof) = self.query_packet_receipt( + let (_, proof_and_height) = self.query_packet_receipt( QueryPacketReceiptRequest { port_id, channel_id, sequence, - height: QueryHeight::Specific(height), + height: QueryHeight::Latest, }, IncludeProof::Yes, )?; - (maybe_packet_proof, None) + let proof_and_height = + proof_and_height.ok_or_else(Error::queried_proof_not_found)?; + (proof_and_height.1, proof_and_height.0, None) } PacketMsgType::TimeoutOrdered => { let (_, maybe_packet_proof) = self.query_next_sequence_receive( QueryNextSequenceReceiveRequest { port_id, channel_id, - height: QueryHeight::Specific(height), + height: QueryHeight::Specific(query_height), }, IncludeProof::Yes, )?; - (maybe_packet_proof, None) + let maybe_packet_proof = + maybe_packet_proof.ok_or_else(Error::queried_proof_not_found)?; + (query_height, maybe_packet_proof, None) } PacketMsgType::TimeoutOnCloseUnordered => { + let (_, packet_and_height) = self.query_packet_receipt( + QueryPacketReceiptRequest { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + sequence, + height: QueryHeight::Latest, + }, + IncludeProof::Yes, + )?; + + let packet_and_height = + packet_and_height.ok_or_else(Error::queried_proof_not_found)?; + let channel_proof = { let (_, maybe_channel_proof) = self.query_channel( QueryChannelRequest { - port_id: port_id.clone(), - channel_id: channel_id.clone(), - height: QueryHeight::Specific(height), + port_id: port_id, + channel_id: channel_id, + height: QueryHeight::Specific(packet_and_height.1), }, IncludeProof::Yes, )?; @@ -651,17 +672,7 @@ pub trait ChainEndpoint: Sized { ) }; - let (_, maybe_packet_proof) = self.query_packet_receipt( - QueryPacketReceiptRequest { - port_id, - channel_id, - sequence, - height: QueryHeight::Specific(height), - }, - IncludeProof::Yes, - )?; - - (maybe_packet_proof, channel_proof) + (packet_and_height.1, packet_and_height.0, channel_proof) } PacketMsgType::TimeoutOnCloseOrdered => { let channel_proof = { @@ -669,7 +680,7 @@ pub trait ChainEndpoint: Sized { QueryChannelRequest { port_id: port_id.clone(), channel_id: channel_id.clone(), - height: QueryHeight::Specific(height), + height: QueryHeight::Specific(query_height), }, IncludeProof::Yes, )?; @@ -687,26 +698,26 @@ pub trait ChainEndpoint: Sized { QueryNextSequenceReceiveRequest { port_id, channel_id, - height: QueryHeight::Specific(height), + height: QueryHeight::Specific(query_height), }, IncludeProof::Yes, )?; - (maybe_packet_proof, channel_proof) + ( + query_height, + maybe_packet_proof.ok_or_else(Error::queried_proof_not_found)?, + channel_proof, + ) } }; - let Some(packet_proof) = maybe_packet_proof else { - return Err(Error::queried_proof_not_found()); - }; - let proofs = Proofs::new( CommitmentProofBytes::try_from(packet_proof).map_err(Error::malformed_proof)?, None, None, None, channel_proof, - height.increment(), + height.increment(), // TODO: do we need to increment (as penumbra already increments when returning the height)? ) .map_err(Error::malformed_proof)?; diff --git a/crates/relayer/src/chain/handle.rs b/crates/relayer/src/chain/handle.rs index 5e4bf0946a..2decc75396 100644 --- a/crates/relayer/src/chain/handle.rs +++ b/crates/relayer/src/chain/handle.rs @@ -334,7 +334,7 @@ pub enum ChainRequest { QueryPacketCommitment { request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - reply_to: ReplyTo<(Vec, Option)>, + reply_to: ReplyTo<(Vec, Option<(MerkleProof, Height)>)>, }, QueryPacketCommitments { @@ -345,7 +345,7 @@ pub enum ChainRequest { QueryPacketReceipt { request: QueryPacketReceiptRequest, include_proof: IncludeProof, - reply_to: ReplyTo<(Vec, Option)>, + reply_to: ReplyTo<(Vec, Option<(MerkleProof, Height)>)>, }, QueryUnreceivedPackets { @@ -356,7 +356,7 @@ pub enum ChainRequest { QueryPacketAcknowledgement { request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - reply_to: ReplyTo<(Vec, Option)>, + reply_to: ReplyTo<(Vec, Option<(MerkleProof, Height)>)>, }, QueryPacketAcknowledgements { @@ -628,7 +628,7 @@ pub trait ChainHandle: Clone + Display + Send + Sync + Debug + 'static { &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error>; + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error>; /// Performs a query to retrieve all the packet commitments hashes /// associated with a channel. Returns the corresponding packet sequence @@ -644,7 +644,7 @@ pub trait ChainHandle: Clone + Display + Send + Sync + Debug + 'static { &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error>; + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error>; /// Performs a query about which IBC packets in the specified list has not /// been received. Returns the sequence numbers of the packets that were not @@ -665,7 +665,7 @@ pub trait ChainHandle: Clone + Display + Send + Sync + Debug + 'static { &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error>; + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error>; /// Performs a query to retrieve all the packet acknowledgements associated /// with a channel. Returns the corresponding packet sequence numbers and diff --git a/crates/relayer/src/chain/handle/base.rs b/crates/relayer/src/chain/handle/base.rs index ef9d171617..08062e0135 100644 --- a/crates/relayer/src/chain/handle/base.rs +++ b/crates/relayer/src/chain/handle/base.rs @@ -452,7 +452,7 @@ impl ChainHandle for BaseChainHandle { &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.send(|reply_to| ChainRequest::QueryPacketCommitment { request, include_proof, @@ -471,7 +471,7 @@ impl ChainHandle for BaseChainHandle { &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.send(|reply_to| ChainRequest::QueryPacketReceipt { request, include_proof, @@ -490,7 +490,7 @@ impl ChainHandle for BaseChainHandle { &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.send(|reply_to| ChainRequest::QueryPacketAcknowledgement { request, include_proof, diff --git a/crates/relayer/src/chain/handle/cache.rs b/crates/relayer/src/chain/handle/cache.rs index e6002537bb..be9e83608c 100644 --- a/crates/relayer/src/chain/handle/cache.rs +++ b/crates/relayer/src/chain/handle/cache.rs @@ -468,7 +468,7 @@ impl ChainHandle for CachingChainHandle { &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.inner().query_packet_commitment(request, include_proof) } @@ -483,7 +483,7 @@ impl ChainHandle for CachingChainHandle { &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.inner().query_packet_receipt(request, include_proof) } @@ -498,7 +498,7 @@ impl ChainHandle for CachingChainHandle { &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.inner() .query_packet_acknowledgement(request, include_proof) } diff --git a/crates/relayer/src/chain/handle/counting.rs b/crates/relayer/src/chain/handle/counting.rs index 9b833eb4af..4ac5f9f389 100644 --- a/crates/relayer/src/chain/handle/counting.rs +++ b/crates/relayer/src/chain/handle/counting.rs @@ -451,7 +451,7 @@ impl ChainHandle for CountingChainHandle { &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.inc_metric("query_packet_commitment"); self.inner().query_packet_commitment(request, include_proof) } @@ -468,7 +468,7 @@ impl ChainHandle for CountingChainHandle { &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.inc_metric("query_packet_receipt"); self.inner().query_packet_receipt(request, include_proof) } @@ -485,7 +485,7 @@ impl ChainHandle for CountingChainHandle { &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.inc_metric("query_packet_acknowledgement"); self.inner() .query_packet_acknowledgement(request, include_proof) diff --git a/crates/relayer/src/chain/runtime.rs b/crates/relayer/src/chain/runtime.rs index f59a2db3c4..770a7d7e82 100644 --- a/crates/relayer/src/chain/runtime.rs +++ b/crates/relayer/src/chain/runtime.rs @@ -740,7 +740,7 @@ where &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - reply_to: ReplyTo<(Vec, Option)>, + reply_to: ReplyTo<(Vec, Option<(MerkleProof, Height)>)>, ) -> Result<(), Error> { let result = self.chain.query_packet_commitment(request, include_proof); reply_to.send(result).map_err(Error::send) @@ -759,7 +759,7 @@ where &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - reply_to: ReplyTo<(Vec, Option)>, + reply_to: ReplyTo<(Vec, Option<(MerkleProof, Height)>)>, ) -> Result<(), Error> { let result = self.chain.query_packet_receipt(request, include_proof); reply_to.send(result).map_err(Error::send) @@ -778,7 +778,7 @@ where &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - reply_to: ReplyTo<(Vec, Option)>, + reply_to: ReplyTo<(Vec, Option<(MerkleProof, Height)>)>, ) -> Result<(), Error> { let result = self .chain diff --git a/crates/relayer/src/transfer.rs b/crates/relayer/src/transfer.rs index 61eed5f722..a86f1e9471 100644 --- a/crates/relayer/src/transfer.rs +++ b/crates/relayer/src/transfer.rs @@ -207,18 +207,14 @@ fn build_transfer_message_astria( let timeout_height = match timeout_height { // TODO: update astria IbcHeight to support optional? - TimeoutHeight::At(height) => { - astria_core::generated::protocol::transaction::v1::IbcHeight { - revision_number: height.revision_number(), - revision_height: height.revision_height(), - } - } - TimeoutHeight::Never => { - astria_core::generated::protocol::transaction::v1::IbcHeight { - revision_number: 0, - revision_height: u64::MAX, - } - } + TimeoutHeight::At(height) => astria_core::generated::protocol::transaction::v1::IbcHeight { + revision_number: height.revision_number(), + revision_height: height.revision_height(), + }, + TimeoutHeight::Never => astria_core::generated::protocol::transaction::v1::IbcHeight { + revision_number: 0, + revision_height: u64::MAX, + }, }; let msg = astria_core::generated::protocol::transaction::v1::Ics20Withdrawal { From 058f8e13d4fea0109e78423cac76639cfd6e6b35 Mon Sep 17 00:00:00 2001 From: elizabeth Date: Fri, 1 Nov 2024 13:34:53 -0400 Subject: [PATCH 2/5] fix tools build --- tools/test-framework/src/relayer/chain.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/test-framework/src/relayer/chain.rs b/tools/test-framework/src/relayer/chain.rs index 68f1a74bf1..1c750fc681 100644 --- a/tools/test-framework/src/relayer/chain.rs +++ b/tools/test-framework/src/relayer/chain.rs @@ -362,7 +362,7 @@ where &self, request: QueryPacketCommitmentRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.value().query_packet_commitment(request, include_proof) } @@ -377,7 +377,7 @@ where &self, request: QueryPacketReceiptRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.value().query_packet_receipt(request, include_proof) } @@ -392,7 +392,7 @@ where &self, request: QueryPacketAcknowledgementRequest, include_proof: IncludeProof, - ) -> Result<(Vec, Option), Error> { + ) -> Result<(Vec, Option<(MerkleProof, Height)>), Error> { self.value() .query_packet_acknowledgement(request, include_proof) } From f261aad0a5218aaaf2c58be074ce66b7976aecb7 Mon Sep 17 00:00:00 2001 From: elizabeth Date: Fri, 1 Nov 2024 14:05:42 -0400 Subject: [PATCH 3/5] decrement astria endpoint resp height --- crates/relayer/src/chain/astria/endpoint.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/relayer/src/chain/astria/endpoint.rs b/crates/relayer/src/chain/astria/endpoint.rs index 841dae794b..6c57e5cc27 100644 --- a/crates/relayer/src/chain/astria/endpoint.rs +++ b/crates/relayer/src/chain/astria/endpoint.rs @@ -1142,7 +1142,12 @@ impl ChainEndpoint for AstriaEndpoint { response.commitment, Some(( proof, - Height::new(height.revision_number, height.revision_height) + // TODO: subtracting from the height is jank but due to penumbra incrementing the proof height + // in the response, and the caller of this method (`build_packet_proofs`) also incrementing the proof + // height. the cosmos endpoint doesn't increment the height in the response (i believe) so changing + // the caller to not increment is potentially more confusing. + // needs a better fix. + Height::new(height.revision_number, height.revision_height - 1) .map_err(|e| Error::other(Box::new(e)))?, )), )) @@ -1226,7 +1231,7 @@ impl ChainEndpoint for AstriaEndpoint { value, Some(( proof, - Height::new(height.revision_number, height.revision_height) + Height::new(height.revision_number, height.revision_height - 1) .map_err(|e| Error::other(Box::new(e)))?, )), )) @@ -1304,7 +1309,7 @@ impl ChainEndpoint for AstriaEndpoint { response.acknowledgement, Some(( proof, - Height::new(height.revision_number, height.revision_height) + Height::new(height.revision_number, height.revision_height - 1) .map_err(|e| Error::other(Box::new(e)))?, )), )) From cec347250646cd11fd26ea5543444f466739a935 Mon Sep 17 00:00:00 2001 From: elizabeth Date: Fri, 1 Nov 2024 14:34:13 -0400 Subject: [PATCH 4/5] update query_next_sequence_receive to also return proof height --- crates/relayer/src/chain/astria/endpoint.rs | 20 +++++++--- crates/relayer/src/chain/cosmos.rs | 5 ++- crates/relayer/src/chain/endpoint.rs | 42 ++++++++++----------- crates/relayer/src/chain/handle.rs | 4 +- crates/relayer/src/chain/handle/base.rs | 2 +- crates/relayer/src/chain/handle/cache.rs | 2 +- crates/relayer/src/chain/handle/counting.rs | 2 +- crates/relayer/src/chain/runtime.rs | 2 +- 8 files changed, 45 insertions(+), 34 deletions(-) diff --git a/crates/relayer/src/chain/astria/endpoint.rs b/crates/relayer/src/chain/astria/endpoint.rs index 6c57e5cc27..5dc63bbb52 100644 --- a/crates/relayer/src/chain/astria/endpoint.rs +++ b/crates/relayer/src/chain/astria/endpoint.rs @@ -1381,7 +1381,7 @@ impl ChainEndpoint for AstriaEndpoint { &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - ) -> Result<(Sequence, Option), Error> { + ) -> Result<(Sequence, Option<(MerkleProof, Height)>), Error> { let mut client = self.ibc_channel_grpc_client.clone(); let request = tonic::Request::new(request.into()); @@ -1391,10 +1391,20 @@ impl ChainEndpoint for AstriaEndpoint { .into_inner(); match include_proof { - IncludeProof::Yes => Ok(( - response.next_sequence_receive.into(), - Some(decode_merkle_proof(response.proof)?), - )), + IncludeProof::Yes => { + let proof = decode_merkle_proof(response.proof)?; + let height = response + .proof_height + .ok_or(Error::grpc_response_param("proof_height".to_string()))?; + Ok(( + response.next_sequence_receive.into(), + Some(( + proof, + Height::new(height.revision_number, height.revision_height - 1) + .map_err(|e| Error::other(Box::new(e)))?, + )), + )) + } IncludeProof::No => Ok((response.next_sequence_receive.into(), None)), } } diff --git a/crates/relayer/src/chain/cosmos.rs b/crates/relayer/src/chain/cosmos.rs index b48c8cb523..9d0c29c556 100644 --- a/crates/relayer/src/chain/cosmos.rs +++ b/crates/relayer/src/chain/cosmos.rs @@ -2086,7 +2086,7 @@ impl ChainEndpoint for CosmosSdkChain { &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - ) -> Result<(Sequence, Option), Error> { + ) -> Result<(Sequence, Option<(MerkleProof, ibc_relayer_types::Height)>), Error> { crate::time!( "query_next_sequence_receive", { @@ -2115,7 +2115,8 @@ impl ChainEndpoint for CosmosSdkChain { let seq: Sequence = Bytes::from(res.value).get_u64().into(); let proof = if prove { - Some(res.proof.ok_or_else(Error::empty_response_proof)?) + let height = ibc_relayer_types::Height::from_tm(res.height, &self.config.id); + Some((res.proof.ok_or_else(Error::empty_response_proof)?, height)) } else { None }; diff --git a/crates/relayer/src/chain/endpoint.rs b/crates/relayer/src/chain/endpoint.rs index 8d0a7172a4..355d0b477e 100644 --- a/crates/relayer/src/chain/endpoint.rs +++ b/crates/relayer/src/chain/endpoint.rs @@ -385,7 +385,7 @@ pub trait ChainEndpoint: Sized { &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - ) -> Result<(Sequence, Option), Error>; + ) -> Result<(Sequence, Option<(MerkleProof, ICSHeight)>), Error>; fn query_txs(&self, request: QueryTxRequest) -> Result, Error>; @@ -625,18 +625,18 @@ pub trait ChainEndpoint: Sized { (proof_and_height.1, proof_and_height.0, None) } PacketMsgType::TimeoutOrdered => { - let (_, maybe_packet_proof) = self.query_next_sequence_receive( + let (_, proof_and_height) = self.query_next_sequence_receive( QueryNextSequenceReceiveRequest { port_id, channel_id, - height: QueryHeight::Specific(query_height), + height: QueryHeight::Latest, }, IncludeProof::Yes, )?; - let maybe_packet_proof = - maybe_packet_proof.ok_or_else(Error::queried_proof_not_found)?; - (query_height, maybe_packet_proof, None) + let proof_and_height = + proof_and_height.ok_or_else(Error::queried_proof_not_found)?; + (proof_and_height.1, proof_and_height.0, None) } PacketMsgType::TimeoutOnCloseUnordered => { let (_, packet_and_height) = self.query_packet_receipt( @@ -675,11 +675,23 @@ pub trait ChainEndpoint: Sized { (packet_and_height.1, packet_and_height.0, channel_proof) } PacketMsgType::TimeoutOnCloseOrdered => { + let (_, packet_and_height) = self.query_next_sequence_receive( + QueryNextSequenceReceiveRequest { + port_id: port_id.clone(), + channel_id: channel_id.clone(), + height: QueryHeight::Latest, + }, + IncludeProof::Yes, + )?; + + let packet_and_height = + packet_and_height.ok_or_else(Error::queried_proof_not_found)?; + let channel_proof = { let (_, maybe_channel_proof) = self.query_channel( QueryChannelRequest { - port_id: port_id.clone(), - channel_id: channel_id.clone(), + port_id, + channel_id, height: QueryHeight::Specific(query_height), }, IncludeProof::Yes, @@ -694,20 +706,8 @@ pub trait ChainEndpoint: Sized { .map_err(Error::malformed_proof)?, ) }; - let (_, maybe_packet_proof) = self.query_next_sequence_receive( - QueryNextSequenceReceiveRequest { - port_id, - channel_id, - height: QueryHeight::Specific(query_height), - }, - IncludeProof::Yes, - )?; - ( - query_height, - maybe_packet_proof.ok_or_else(Error::queried_proof_not_found)?, - channel_proof, - ) + (packet_and_height.1, packet_and_height.0, channel_proof) } }; diff --git a/crates/relayer/src/chain/handle.rs b/crates/relayer/src/chain/handle.rs index 2decc75396..20493eddad 100644 --- a/crates/relayer/src/chain/handle.rs +++ b/crates/relayer/src/chain/handle.rs @@ -312,7 +312,7 @@ pub enum ChainRequest { QueryNextSequenceReceive { request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - reply_to: ReplyTo<(Sequence, Option)>, + reply_to: ReplyTo<(Sequence, Option<(MerkleProof, Height)>)>, }, BuildChannelProofs { @@ -546,7 +546,7 @@ pub trait ChainHandle: Clone + Display + Send + Sync + Debug + 'static { &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - ) -> Result<(Sequence, Option), Error>; + ) -> Result<(Sequence, Option<(MerkleProof, Height)>), Error>; /// Performs a query to retrieve all the channels of a chain. fn query_channels( diff --git a/crates/relayer/src/chain/handle/base.rs b/crates/relayer/src/chain/handle/base.rs index 08062e0135..87c2836403 100644 --- a/crates/relayer/src/chain/handle/base.rs +++ b/crates/relayer/src/chain/handle/base.rs @@ -312,7 +312,7 @@ impl ChainHandle for BaseChainHandle { &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - ) -> Result<(Sequence, Option), Error> { + ) -> Result<(Sequence, Option<(MerkleProof, Height)>), Error> { self.send(|reply_to| ChainRequest::QueryNextSequenceReceive { request, include_proof, diff --git a/crates/relayer/src/chain/handle/cache.rs b/crates/relayer/src/chain/handle/cache.rs index be9e83608c..2efcdfd9ef 100644 --- a/crates/relayer/src/chain/handle/cache.rs +++ b/crates/relayer/src/chain/handle/cache.rs @@ -339,7 +339,7 @@ impl ChainHandle for CachingChainHandle { &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - ) -> Result<(Sequence, Option), Error> { + ) -> Result<(Sequence, Option<(MerkleProof, Height)>), Error> { self.inner() .query_next_sequence_receive(request, include_proof) } diff --git a/crates/relayer/src/chain/handle/counting.rs b/crates/relayer/src/chain/handle/counting.rs index 4ac5f9f389..40da2f5a79 100644 --- a/crates/relayer/src/chain/handle/counting.rs +++ b/crates/relayer/src/chain/handle/counting.rs @@ -334,7 +334,7 @@ impl ChainHandle for CountingChainHandle { &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - ) -> Result<(Sequence, Option), Error> { + ) -> Result<(Sequence, Option<(MerkleProof, Height)>), Error> { self.inc_metric("query_next_sequence_receive"); self.inner() .query_next_sequence_receive(request, include_proof) diff --git a/crates/relayer/src/chain/runtime.rs b/crates/relayer/src/chain/runtime.rs index 770a7d7e82..2db2db6d7f 100644 --- a/crates/relayer/src/chain/runtime.rs +++ b/crates/relayer/src/chain/runtime.rs @@ -808,7 +808,7 @@ where &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - reply_to: ReplyTo<(Sequence, Option)>, + reply_to: ReplyTo<(Sequence, Option<(MerkleProof, Height)>)>, ) -> Result<(), Error> { let result = self .chain From 19b738a12c1b3cc803ce85a2a0f2dd1b5bd3aa7c Mon Sep 17 00:00:00 2001 From: elizabeth Date: Fri, 1 Nov 2024 14:38:56 -0400 Subject: [PATCH 5/5] fix tools --- tools/test-framework/src/relayer/chain.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/test-framework/src/relayer/chain.rs b/tools/test-framework/src/relayer/chain.rs index 1c750fc681..659afba851 100644 --- a/tools/test-framework/src/relayer/chain.rs +++ b/tools/test-framework/src/relayer/chain.rs @@ -256,7 +256,7 @@ where &self, request: QueryNextSequenceReceiveRequest, include_proof: IncludeProof, - ) -> Result<(Sequence, Option), Error> { + ) -> Result<(Sequence, Option<(MerkleProof, Height)>), Error> { self.value() .query_next_sequence_receive(request, include_proof) }