Skip to content

Commit

Permalink
ibc: delete packet commitments instead of writing empty values
Browse files Browse the repository at this point in the history
The IBC spec calls for deletion of packet commitments after a successful acknowledgement, in order to avoid double-acknowledgements.

Previously, our implementation accomplished this deletion by writing an empty byte slice vec![] to the commitment path.

However, this will cause nonexistence proofs by exclusion (the standard NX-proof type used in ics23) of adjacent keys to fail, since ics23 does not support existence proofs of empty values.

This PR changes `delete_packet_commitment` to actually delete the commitment, and introduces a migration for testnet 78 that will delete all of the previously overwritten commitments.
  • Loading branch information
avahowell committed Jun 19, 2024
1 parent 1693c20 commit f7f6d2b
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
34 changes: 34 additions & 0 deletions crates/bin/pd/src/migrate/testnet78.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
//! Contains functions related to the migration script of Testnet78.
use cnidarium::StateRead;
use cnidarium::{Snapshot, StateDelta, StateWrite, Storage};
use futures::TryStreamExt as _;
use futures::{pin_mut, StreamExt};
Expand Down Expand Up @@ -89,6 +90,9 @@ pub async fn migrate(
// Write the new dex parameters (with the execution budget field) to the state.
update_dex_params(&mut delta).await?;

// Migrate empty (deleted) packet commitments to be deleted at the tree level
delete_empty_deleted_packet_commitments(&mut delta).await?;

// Reset the application height and halt flag.
delta.ready_to_start();
delta.put_block_height(0u64);
Expand Down Expand Up @@ -151,6 +155,36 @@ pub async fn migrate(
Ok(())
}

async fn delete_empty_deleted_packet_commitments(
delta: &mut StateDelta<Snapshot>,
) -> anyhow::Result<()> {
let prefix_key = "ibc-data/commitments/";
let stream = delta.prefix_raw(&prefix_key);

let stream_empty_commitments = stream.filter_map(|entry| async {
match entry {
Ok((key, val)) => {
if val.is_empty() {
Some(key)
} else {
None
}
}
Err(e) => {
tracing::error!(?e, "error reading packet commitment");
None
}
}
});
pin_mut!(stream_empty_commitments);

while let Some(key) = stream_empty_commitments.next().await {
delta.delete(key);
}

Ok(())
}

/// Write the updated dex parameters to the chain state.
async fn update_dex_params(delta: &mut StateDelta<Snapshot>) -> anyhow::Result<()> {
let mut dex_params = delta
Expand Down
3 changes: 1 addition & 2 deletions crates/core/component/ibc/src/component/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,10 @@ pub trait StateWriteExt: StateWrite + StateReadExt {
port_id: &PortId,
sequence: u64,
) {
self.put_raw(
self.delete(
IBC_COMMITMENT_PREFIX.apply_string(
CommitmentPath::new(port_id, channel_id, sequence.into()).to_string(),
),
vec![],
);
}

Expand Down

0 comments on commit f7f6d2b

Please sign in to comment.