Skip to content

Commit 74f4e77

Browse files
sanityclaude
andcommitted
fix: handle Unknown addresses in get.rs to avoid panics
When processing ReturnGet with empty value, the code previously assumed sender_from_addr was always Some and that addresses were always Known. This caused panics in CI: 1. expect() on None sender_from_addr 2. .peer() called on PeerKeyLocation with Unknown address Changes: - Return error gracefully when sender lookup fails - Use pub_key() instead of peer() for logging - Guard tried_peers insertions with socket_addr() checks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 31a6d9a commit 74f4e77

File tree

1 file changed

+24
-12
lines changed
  • crates/core/src/operations

1 file changed

+24
-12
lines changed

crates/core/src/operations/get.rs

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -854,16 +854,23 @@ impl Operation for GetOp {
854854
let id = *id;
855855
let key = *key;
856856

857-
// Use sender_from_addr for logging
858-
let sender = sender_from_addr.clone().expect(
859-
"ReturnGet requires sender lookup from connection - source_addr should resolve to known peer",
860-
);
857+
// Handle case where sender lookup failed (e.g., peer disconnected)
858+
let Some(sender) = sender_from_addr.clone() else {
859+
tracing::warn!(
860+
tx = %id,
861+
%key,
862+
source = ?source_addr,
863+
"GET: ReturnGet (empty) received but sender lookup failed - cannot process"
864+
);
865+
return Err(OpError::invalid_transition(self.id));
866+
};
861867

868+
// Use pub_key for logging to avoid panics on Unknown addresses
862869
tracing::info!(
863870
tx = %id,
864871
%key,
865-
from = %sender.peer(),
866-
to = %target.peer(),
872+
from = %sender.pub_key(),
873+
to = %target.pub_key(),
867874
skip = ?skip_list,
868875
"GET: ReturnGet received with empty value"
869876
);
@@ -875,7 +882,7 @@ impl Operation for GetOp {
875882
%this_peer,
876883
"Neither contract or contract value for contract found at peer {}, \
877884
retrying with other peers",
878-
sender.peer()
885+
sender.pub_key()
879886
);
880887

881888
match self.state {
@@ -894,8 +901,10 @@ impl Operation for GetOp {
894901
}) => {
895902
// todo: register in the stats for the outcome of the op that failed to get a response from this peer
896903

897-
// Add the failed peer to tried list
898-
tried_peers.insert(sender.peer().clone());
904+
// Add the failed peer to tried list (only if address is known)
905+
if let Some(addr) = sender.socket_addr() {
906+
tried_peers.insert(PeerId::new(addr, sender.pub_key().clone()));
907+
}
899908

900909
// First, check if we have alternatives at this hop level
901910
if !alternatives.is_empty() && attempts_at_hop < DEFAULT_MAX_BREADTH {
@@ -905,7 +914,7 @@ impl Operation for GetOp {
905914
tracing::info!(
906915
tx = %id,
907916
%key,
908-
next_peer = %next_target.peer(),
917+
next_peer = %next_target.pub_key(),
909918
fetch_contract,
910919
attempts_at_hop = attempts_at_hop + 1,
911920
max_attempts = DEFAULT_MAX_BREADTH,
@@ -923,8 +932,11 @@ impl Operation for GetOp {
923932
skip_list: tried_peers.clone(),
924933
});
925934

926-
// Update state with the new alternative being tried
927-
tried_peers.insert(next_target.peer().clone());
935+
// Update state with the new alternative being tried (only if address is known)
936+
if let Some(addr) = next_target.socket_addr() {
937+
tried_peers
938+
.insert(PeerId::new(addr, next_target.pub_key().clone()));
939+
}
928940
let updated_tried_peers = tried_peers.clone();
929941
new_state = Some(GetState::AwaitingResponse {
930942
retries,

0 commit comments

Comments
 (0)