From 5da70de9bfa9f89c16ca5b464b74313f2b561239 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Wed, 1 Nov 2023 06:22:39 +0100 Subject: [PATCH 1/7] perf: avoid temporary Vec allocations when computing commitments --- .../unreleased/improvements/939-less-alloc.md | 2 + .../ibc/src/core/ics04_channel/commitment.rs | 42 +++++++++++++++---- 2 files changed, 35 insertions(+), 9 deletions(-) create mode 100644 .changelog/unreleased/improvements/939-less-alloc.md diff --git a/.changelog/unreleased/improvements/939-less-alloc.md b/.changelog/unreleased/improvements/939-less-alloc.md new file mode 100644 index 000000000..03b7e5909 --- /dev/null +++ b/.changelog/unreleased/improvements/939-less-alloc.md @@ -0,0 +1,2 @@ +- Reduce amount of temporary vector allocations. + ([\#939](https://github.com/cosmos/ibc-rs/pull/939)) diff --git a/crates/ibc/src/core/ics04_channel/commitment.rs b/crates/ibc/src/core/ics04_channel/commitment.rs index c2979e910..d9b464c8a 100644 --- a/crates/ibc/src/core/ics04_channel/commitment.rs +++ b/crates/ibc/src/core/ics04_channel/commitment.rs @@ -86,16 +86,16 @@ pub(crate) fn compute_packet_commitment( timeout_height: &TimeoutHeight, timeout_timestamp: &Timestamp, ) -> PacketCommitment { - let mut hash_input = timeout_timestamp.nanoseconds().to_be_bytes().to_vec(); + use sha2::Digest; - let revision_number = timeout_height.commitment_revision_number().to_be_bytes(); - hash_input.append(&mut revision_number.to_vec()); + let mut hash_input = [0; 3 * 8 + 32]; - let revision_height = timeout_height.commitment_revision_height().to_be_bytes(); - hash_input.append(&mut revision_height.to_vec()); + hash_input[..8].copy_from_slice(&timeout_timestamp.nanoseconds().to_be_bytes()); + hash_input[8..16].copy_from_slice(&timeout_height.commitment_revision_number().to_be_bytes()); + hash_input[16..24].copy_from_slice(&timeout_height.commitment_revision_height().to_be_bytes()); - let packet_data_hash = hash(packet_data); - hash_input.append(&mut packet_data_hash.to_vec()); + let packet_data_hash = sha2::Sha256::digest(packet_data); + hash_input[24..].copy_from_slice(packet_data_hash.as_slice()); hash(&hash_input).into() } @@ -109,8 +109,32 @@ pub(crate) fn compute_ack_commitment(ack: &Acknowledgement) -> AcknowledgementCo /// /// Note that computing commitments with anything other than SHA256 will /// break the Merkle proofs of the IBC provable store. -fn hash(data: impl AsRef<[u8]>) -> Vec { +fn hash(data: &[u8]) -> Vec { use sha2::Digest; - sha2::Sha256::digest(&data).to_vec() + sha2::Sha256::digest(data).to_vec() +} + +#[test] +fn test_compute_commitment() { + // PacketCommitment + let want: [u8; 32] = [ + 169, 40, 181, 31, 98, 189, 84, 0, 145, 236, 69, 31, 78, 243, 69, 121, 79, 5, 158, 101, 145, + 8, 22, 134, 97, 38, 220, 54, 79, 132, 204, 21, + ]; + let got = compute_packet_commitment( + "packet data".as_bytes(), + &TimeoutHeight::At(crate::Height::new(42, 24).unwrap()), + &Timestamp::from_nanoseconds(0x42).unwrap(), + ); + assert_eq!(&want[..], got.as_ref()); + + // AcknowledgementCommitment + let want: [u8; 32] = [ + 5, 78, 222, 193, 208, 33, 31, 98, 79, 237, 12, 188, 169, 212, 249, 64, 11, 14, 73, 28, 67, + 116, 42, 242, 197, 176, 171, 235, 240, 201, 144, 216, + ]; + let ack = Acknowledgement::try_from(vec![0u8, 1, 2, 3]).unwrap(); + let got = compute_ack_commitment(&ack); + assert_eq!(&want[..], got.as_ref()) } From 4a4e51560356541dbc87df775463b014dfd99f9d Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Thu, 2 Nov 2023 18:12:44 +0100 Subject: [PATCH 2/7] review --- crates/ibc/src/core/ics04_channel/commitment.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/commitment.rs b/crates/ibc/src/core/ics04_channel/commitment.rs index d9b464c8a..2f63c841b 100644 --- a/crates/ibc/src/core/ics04_channel/commitment.rs +++ b/crates/ibc/src/core/ics04_channel/commitment.rs @@ -86,33 +86,29 @@ pub(crate) fn compute_packet_commitment( timeout_height: &TimeoutHeight, timeout_timestamp: &Timestamp, ) -> PacketCommitment { - use sha2::Digest; - let mut hash_input = [0; 3 * 8 + 32]; hash_input[..8].copy_from_slice(&timeout_timestamp.nanoseconds().to_be_bytes()); hash_input[8..16].copy_from_slice(&timeout_height.commitment_revision_number().to_be_bytes()); hash_input[16..24].copy_from_slice(&timeout_height.commitment_revision_height().to_be_bytes()); + hash_input[24..].copy_from_slice(&hash(packet_data)); - let packet_data_hash = sha2::Sha256::digest(packet_data); - hash_input[24..].copy_from_slice(packet_data_hash.as_slice()); - - hash(&hash_input).into() + PacketCommitment(hash(&hash_input).into()) } /// Compute the commitment for an acknowledgement. pub(crate) fn compute_ack_commitment(ack: &Acknowledgement) -> AcknowledgementCommitment { - hash(ack.as_ref()).into() + AcknowledgementCommitment(hash(ack.as_ref()).into()) } /// Helper function to hash a byte slice using SHA256. /// /// Note that computing commitments with anything other than SHA256 will /// break the Merkle proofs of the IBC provable store. -fn hash(data: &[u8]) -> Vec { +fn hash(data: &[u8]) -> [u8; 32] { use sha2::Digest; - sha2::Sha256::digest(data).to_vec() + sha2::Sha256::digest(data).into() } #[test] From cd04eb59f05194614d038224422354a73b79dda7 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Thu, 2 Nov 2023 19:45:23 +0100 Subject: [PATCH 3/7] Update crates/ibc/src/core/ics04_channel/commitment.rs Co-authored-by: Rano | Ranadeep Signed-off-by: Michal Nazarewicz --- crates/ibc/src/core/ics04_channel/commitment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/core/ics04_channel/commitment.rs b/crates/ibc/src/core/ics04_channel/commitment.rs index 2f63c841b..abcbed422 100644 --- a/crates/ibc/src/core/ics04_channel/commitment.rs +++ b/crates/ibc/src/core/ics04_channel/commitment.rs @@ -98,7 +98,7 @@ pub(crate) fn compute_packet_commitment( /// Compute the commitment for an acknowledgement. pub(crate) fn compute_ack_commitment(ack: &Acknowledgement) -> AcknowledgementCommitment { - AcknowledgementCommitment(hash(ack.as_ref()).into()) + hash(ack.as_ref()).to_vec().into() } /// Helper function to hash a byte slice using SHA256. From 981f9b649a9a5113127cd896ec4ed96f066a1ccc Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Thu, 2 Nov 2023 19:45:32 +0100 Subject: [PATCH 4/7] Update crates/ibc/src/core/ics04_channel/commitment.rs Co-authored-by: Rano | Ranadeep Signed-off-by: Michal Nazarewicz --- crates/ibc/src/core/ics04_channel/commitment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/core/ics04_channel/commitment.rs b/crates/ibc/src/core/ics04_channel/commitment.rs index abcbed422..72f43f825 100644 --- a/crates/ibc/src/core/ics04_channel/commitment.rs +++ b/crates/ibc/src/core/ics04_channel/commitment.rs @@ -93,7 +93,7 @@ pub(crate) fn compute_packet_commitment( hash_input[16..24].copy_from_slice(&timeout_height.commitment_revision_height().to_be_bytes()); hash_input[24..].copy_from_slice(&hash(packet_data)); - PacketCommitment(hash(&hash_input).into()) + hash(&hash_input).to_vec().into() } /// Compute the commitment for an acknowledgement. From 34f07da573f2e82da8a1f78873f60dbd8a87861b Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Thu, 2 Nov 2023 19:45:39 +0100 Subject: [PATCH 5/7] Update crates/ibc/src/core/ics04_channel/commitment.rs Co-authored-by: Rano | Ranadeep Signed-off-by: Michal Nazarewicz --- crates/ibc/src/core/ics04_channel/commitment.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ibc/src/core/ics04_channel/commitment.rs b/crates/ibc/src/core/ics04_channel/commitment.rs index 72f43f825..769ddb5f0 100644 --- a/crates/ibc/src/core/ics04_channel/commitment.rs +++ b/crates/ibc/src/core/ics04_channel/commitment.rs @@ -86,7 +86,7 @@ pub(crate) fn compute_packet_commitment( timeout_height: &TimeoutHeight, timeout_timestamp: &Timestamp, ) -> PacketCommitment { - let mut hash_input = [0; 3 * 8 + 32]; + let mut hash_input = [0; 8 * 3 + 32]; hash_input[..8].copy_from_slice(&timeout_timestamp.nanoseconds().to_be_bytes()); hash_input[8..16].copy_from_slice(&timeout_height.commitment_revision_number().to_be_bytes()); From 53663b1e736d919df168017c4bc16ac9fb346631 Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Thu, 2 Nov 2023 19:50:14 +0100 Subject: [PATCH 6/7] review --- .../ibc/src/core/ics04_channel/commitment.rs | 52 +++++++++++-------- 1 file changed, 30 insertions(+), 22 deletions(-) diff --git a/crates/ibc/src/core/ics04_channel/commitment.rs b/crates/ibc/src/core/ics04_channel/commitment.rs index 769ddb5f0..8903f310a 100644 --- a/crates/ibc/src/core/ics04_channel/commitment.rs +++ b/crates/ibc/src/core/ics04_channel/commitment.rs @@ -111,26 +111,34 @@ fn hash(data: &[u8]) -> [u8; 32] { sha2::Sha256::digest(data).into() } -#[test] -fn test_compute_commitment() { - // PacketCommitment - let want: [u8; 32] = [ - 169, 40, 181, 31, 98, 189, 84, 0, 145, 236, 69, 31, 78, 243, 69, 121, 79, 5, 158, 101, 145, - 8, 22, 134, 97, 38, 220, 54, 79, 132, 204, 21, - ]; - let got = compute_packet_commitment( - "packet data".as_bytes(), - &TimeoutHeight::At(crate::Height::new(42, 24).unwrap()), - &Timestamp::from_nanoseconds(0x42).unwrap(), - ); - assert_eq!(&want[..], got.as_ref()); - - // AcknowledgementCommitment - let want: [u8; 32] = [ - 5, 78, 222, 193, 208, 33, 31, 98, 79, 237, 12, 188, 169, 212, 249, 64, 11, 14, 73, 28, 67, - 116, 42, 242, 197, 176, 171, 235, 240, 201, 144, 216, - ]; - let ack = Acknowledgement::try_from(vec![0u8, 1, 2, 3]).unwrap(); - let got = compute_ack_commitment(&ack); - assert_eq!(&want[..], got.as_ref()) +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_compute_packet_commitment() { + let expected: [u8; 32] = [ + 0xa9, 0x28, 0xb5, 0x1f, 0x62, 0xbd, 0x54, 0x00, 0x91, 0xec, 0x45, 0x1f, 0x4e, 0xf3, + 0x45, 0x79, 0x4f, 0x05, 0x9e, 0x65, 0x91, 0x08, 0x16, 0x86, 0x61, 0x26, 0xdc, 0x36, + 0x4f, 0x84, 0xcc, 0x15, + ]; + let actual = compute_packet_commitment( + "packet data".as_bytes(), + &TimeoutHeight::At(crate::Height::new(42, 24).unwrap()), + &Timestamp::from_nanoseconds(0x42).unwrap(), + ); + assert_eq!(&expected[..], actual.as_ref()); + } + + #[test] + fn test_compute_ack_commitment() { + let expected: [u8; 32] = [ + 0x05, 0x4e, 0xde, 0xc1, 0xd0, 0x21, 0x1f, 0x62, 0x4f, 0xed, 0x0c, 0xbc, 0xa9, 0xd4, + 0xf9, 0x40, 0x0b, 0x0e, 0x49, 0x1c, 0x43, 0x74, 0x2a, 0xf2, 0xc5, 0xb0, 0xab, 0xeb, + 0xf0, 0xc9, 0x90, 0xd8, + ]; + let ack = Acknowledgement::try_from(vec![0, 1, 2, 3]).unwrap(); + let actual = compute_ack_commitment(&ack); + assert_eq!(&expected[..], actual.as_ref()) + } } From 670c4b23a607c2aa61b481e051334f6fa5297c0a Mon Sep 17 00:00:00 2001 From: Michal Nazarewicz Date: Thu, 2 Nov 2023 19:50:58 +0100 Subject: [PATCH 7/7] Update .changelog/unreleased/improvements/939-less-alloc.md Co-authored-by: Rano | Ranadeep Signed-off-by: Michal Nazarewicz --- .changelog/unreleased/improvements/939-less-alloc.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changelog/unreleased/improvements/939-less-alloc.md b/.changelog/unreleased/improvements/939-less-alloc.md index 03b7e5909..b7ed133f9 100644 --- a/.changelog/unreleased/improvements/939-less-alloc.md +++ b/.changelog/unreleased/improvements/939-less-alloc.md @@ -1,2 +1,2 @@ -- Reduce amount of temporary vector allocations. +- Reduce vector allocations in Commitment computation. ([\#939](https://github.com/cosmos/ibc-rs/pull/939))