Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up packets clearing logic in RelayPath #1879

Merged
merged 3 commits into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 9 additions & 33 deletions relayer/src/link/relay_path.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use alloc::collections::BTreeMap as HashMap;
use alloc::collections::VecDeque;
use std::sync::{Arc, RwLock};
use std::thread;
use std::time::Instant;

Expand Down Expand Up @@ -51,7 +50,6 @@ use crate::link::pending::PendingTxs;
use crate::link::relay_sender::{AsyncReply, SubmitReply};
use crate::link::relay_summary::RelaySummary;
use crate::link::{pending, relay_sender};
use crate::util::lock::LockExt;
use crate::util::queue::Queue;

const MAX_RETRIES: usize = 5;
Expand All @@ -65,11 +63,6 @@ pub struct RelayPath<ChainA: ChainHandle, ChainB: ChainHandle> {
dst_channel_id: ChannelId,
dst_port_id: PortId,

// Marks whether this path has already cleared pending packets.
// Packets should be cleared once (at startup), then this
// flag turns to `false`.
clear_packets: Arc<RwLock<bool>>,

// Operational data, targeting both the source and destination chain.
// These vectors of operational data are ordered decreasingly by
// their age, with element at position `0` being the oldest.
Expand Down Expand Up @@ -122,7 +115,6 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
dst_channel_id: dst_channel_id.clone(),
dst_port_id: dst_port_id.clone(),

clear_packets: Arc::new(RwLock::new(true)),
src_operational_data: Queue::new(),
dst_operational_data: Queue::new(),

Expand Down Expand Up @@ -309,34 +301,18 @@ impl<ChainA: ChainHandle, ChainB: ChainHandle> RelayPath<ChainA, ChainB> {
Err(LinkError::old_packet_clearing_failed())
}

fn should_clear_packets(&self) -> bool {
*self.clear_packets.acquire_read()
}

/// Clears any packets that were sent before `height`, either if the `clear_packets` flag
/// is set or if clearing is forced by the caller.
pub fn schedule_packet_clearing(
&self,
height: Option<Height>,
force: bool,
) -> Result<(), LinkError> {
if self.should_clear_packets() || force {
let span = span!(Level::DEBUG, "clear", f = ?force);
let _enter = span.enter();

// Disable further clearing of old packets by default.
// Clearing may still happen: upon new blocks, when `force = true`.
*self.clear_packets.acquire_write() = false;

let clear_height = height
.map(|h| h.decrement().map_err(|e| LinkError::decrement_height(h, e)))
.transpose()?;
/// Clears any packets that were sent before `height`.
pub fn schedule_packet_clearing(&self, height: Option<Height>) -> Result<(), LinkError> {
let span = span!(Level::DEBUG, "clear");
let _enter = span.enter();

self.relay_pending_packets(clear_height)?;
let clear_height = height
.map(|h| h.decrement().map_err(|e| LinkError::decrement_height(h, e)))
.transpose()?;

debug!(height = ?clear_height, "done scheduling");
}
self.relay_pending_packets(clear_height)?;

debug!(height = ?clear_height, "done scheduling");
Ok(())
}

Expand Down
17 changes: 9 additions & 8 deletions relayer/src/worker/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,16 +148,17 @@ fn handle_packet_cmd<ChainA: ChainHandle, ChainB: ChainHandle>(
height,
new_block: _,
} => {
let do_clear_packet =
should_clear_packets(is_first_run, clear_on_start, clear_interval, height);

// Schedule the clearing of pending packets. This may happen once at start,
// and may be _forced_ at predefined block intervals.
link.a_to_b
.schedule_packet_clearing(Some(height), do_clear_packet)
// Decide if packet clearing should be scheduled.
// Packet clearing may happen once at start,
// and then at predefined block intervals.
if should_clear_packets(is_first_run, clear_on_start, clear_interval, height) {
link.a_to_b.schedule_packet_clearing(Some(height))
} else {
Ok(())
}
}

WorkerCmd::ClearPendingPackets => link.a_to_b.schedule_packet_clearing(None, true),
WorkerCmd::ClearPendingPackets => link.a_to_b.schedule_packet_clearing(None),
};

if let Err(e) = result {
Expand Down