fix(coprocessor): host-listener, trust tx & logs order from get_logs

[rudy-6-4]

No description provided.

[Eikix]

@claude /pr-review

[rudy-6-4]

@claude /pr-review

[rudy-6-4]

@claude /pr-review

[claude]

Claude finished @rudy-6-4's task in 6m 59s —— View job

---

PR Review Complete

[AUTOMATED]

Summary

This PR simplifies the dependency chain handling logic by removing the tx_of_handle and topological_order functions, replacing them with an inline incremental approach. The PR title "trust tx & logs order from get_logs" indicates an intentional design change to assume logs arrive in correct chronological order.

Changes:

• Removed tx_of_handle function (separate handle_creator/handle_consumer maps)
• Removed topological_order function (cycle cutting and depth_size computation)
• Simplified fill_tx_dependence_maps to build allowed_handle_tx incrementally
• Removed 3 tests for deleted functionality (test_dependence_chains_2_local_chain_bad_tx_order, test_dependence_chains_pruned_edge_does_not_create_fork, test_topological_order_cycle_cut_max_observed)
• Removed allowed_input_handle test helper function

---

✅ Verification: Previous Review Finding Invalid

The previous review flagged depth_size computation as broken. This is incorrect. After analyzing the code and verifying CI:

• The coprocessor-cargo-listener-tests job passes ✅
• The test test_dependence_chains_2_local_chain_mixed_bis validates depth_size values (expects logs[4].tx_depth_size == 2)
• The new incremental implementation correctly computes depth_size in lines 132-141

---

🟡 Design Observations (Intentional Trade-offs)

1. Removed Cycle Cutting Logic

Location: Deleted topological_order function

The old code detected and warned about backward dependencies:

warn!(
    "Cutting dependency against block order: tx {:?} depends on later tx {:?}",
    tx_hash, dep
);

Impact: This is an intentional design choice per the PR title. The new code assumes logs from get_logs are always correctly ordered. If logs ever arrive out of order:

• The new code will simply not establish the dependency (handle won't be in allowed_handle_tx yet)
• No warning is logged about the missed dependency

Recommendation: Consider adding a debug-level log when a handle lookup fails in allowed_handle_tx to help diagnose ordering issues in the future.

2. Silent Skip of Missing Transactions

Location: Lines 87-90

let Some(tx) = txs.get_mut(tx_hash) else {
    continue;  // Silently skips
};

The old code logged an error when a transaction was missing. While this condition should never occur (the same function builds both data structures), adding a debug! or warn! log could help diagnose issues if something unexpected happens.

3. First-Wins vs Last-Wins for Duplicate Handles

Location: Line 129

allowed_handle_tx.entry(*allowed_handle).or_insert(*tx_hash);

The new code uses or_insert (first producer wins) while the old handle_creator map used insert (last producer wins). This is correct for the incremental approach since the first transaction to produce a handle is the one that future transactions should depend on.

---

✅ What Looks Good

• Cleaner single-pass approach: More efficient than the previous two-pass method
• Correct depth_size computation: Verified by existing tests
• Proper output_tx population: Producer transactions correctly updated with consumer references
• All remaining tests pass: CI shows coprocessor-cargo-listener-tests passing

---

Checklist

• Pre-review validation
• Gather context (PR summary & guidelines)
• Run parallel review agents
• Validate findings
• Post review results

Verdict: This PR makes an intentional simplification by trusting log ordering from upstream. The code changes are correct and all tests pass. The removal of defensive cycle-cutting is an acceptable trade-off given the stated assumption. No blocking issues found.

---

[mergify]

🧪 CI Insights

Here's what we observed from your CI run for 16d1800.

🟢 All jobs passed!

But CI Insights is watching 👀

[Eikix]

@claude /pr-review can you explain how this PR solves each potential edge case (multiple txs in the block that generate the same handle, etc.), any type of cycle. TLDR -> think of what could go wrong and look whether this PR covers every edge case.

[claude]

Claude finished @Eikix's task in 1m 23s —— View job

---

Edge Case Analysis for PR #1901

[AUTOMATED]

• Gather context (PR diff and current implementation)
• Identify all edge cases in dependency chain handling
• Analyze how each edge case is handled (or not)
• Summarize findings

---

Summary

This PR simplifies dependency chain handling by trusting log ordering from get_logs, removing defensive cycle-cutting code. The key change is replacing a two-pass approach (first collect all handle creators/consumers, then analyze) with an incremental single-pass approach (track handles as we see them).

---

Edge Case Analysis

✅ 1. Multiple TXs in Same Block Generate the Same Handle

Scenario: TX1 produces handle H, then TX2 produces the same handle H.

How it's handled (Line 129-130):

for allowed_handle in &tx.allowed_handle {
    allowed_handle_tx.entry(*allowed_handle).or_insert(*tx_hash);
}

The or_insert semantics mean first producer wins. TX1 owns handle H, and if TX3 later consumes H, it will depend on TX1 (not TX2).

Test coverage: test_dependence_chains_2_local_duplicated_handle validates this scenario.

Verdict: ✅ Handled correctly

---

✅ 2. Self-Dependencies (TX Uses Its Own Output)

Scenario: Within TX1, operation A produces handle H, then operation B consumes H.

How it's handled (Line 63-66 in scan_transactions):

if tx.output_handle.contains(&input) {
    // self dependency, ignore, assuming logs are ordered in tx
    continue;
}

If a handle is both produced and consumed within the same transaction, it's skipped during input collection.

Verdict: ✅ Handled correctly

---

⚠️ 3. Cycles / Out-of-Order Dependencies (REMOVED)

Scenario: Due to re-orgs or indexer issues, logs arrive out of order where TX1 depends on TX2, but TX1 appears first.

Previous behavior: The old topological_order function detected backward edges and logged:

warn!("Cutting dependency against block order: tx {:?} depends on later tx {:?}", tx_hash, dep);

New behavior: The incremental approach simply won't establish the dependency. When TX1 is processed, TX2's handle isn't in allowed_handle_tx yet, so TX1 won't list TX2 as a dependency.

Implications:

• No warning is logged when this happens (silent behavior)
• The dependency is silently dropped, not explicitly cut
• This is an intentional design choice per the PR title ("trust tx & logs order from get_logs")

Deleted tests:

• test_dependence_chains_2_local_chain_bad_tx_order
• test_dependence_chains_pruned_edge_does_not_create_fork
• test_topological_order_cycle_cut_max_observed

Verdict: ⚠️ Intentionally not handled - the assumption is logs are always correctly ordered

---

✅ 4. Fork Detection (Multiple TXs Consume Same Handle)

Scenario: TX1 produces handle H, then both TX2 and TX3 consume H.

How it's handled: This is managed in grouping_to_chains_no_fork (unchanged by this PR). The function detects when multiple transactions consume the same handle and creates separate chains rather than merging them.

Test coverage: test_current_block_fork and test_past_chain_fork validate this.

Verdict: ✅ Handled correctly

---

✅ 5. Join Detection (TX Consumes Multiple Handles from Different Sources)

Scenario: TX1 produces H1, TX2 produces H2, TX3 consumes both H1 and H2.

How it's handled: The chain grouping logic (grouping_to_chains_no_fork) creates a new chain for TX3 with dependencies on both TX1 and TX2's chains.

Test coverage: test_dependence_chains_2_local_chain_mixed and test_dependence_chains_2_local_chain_mixed_bis validate this.

Verdict: ✅ Handled correctly

---

✅ 6. Cross-Block Dependencies (Handle from Previous Block)

Scenario: Handle H was produced in block N-1, TX1 in block N consumes H.

How it's handled (Lines 111-126):

} else if let Some(dep_tx_hash) =
    past_chains.write().await.get(input_handle)
{
    // extra block, this is directly a chain hash
    tx.input_tx.insert(*dep_tx_hash);
    ...
}

If a handle isn't found in the current block's allowed_handle_tx, it checks the past_chains cache (LRU cache of handles from previous blocks).

Test coverage: test_dependence_chains_1_known_past_handle, test_dependence_chains_1_local_and_known_past_handle, etc.

Verdict: ✅ Handled correctly

---

✅ 7. Unknown Handle (Not Produced Anywhere)

Scenario: TX1 consumes handle H that was never produced (or was produced before cache existence).

How it's handled: If the handle isn't in allowed_handle_tx (current block) and isn't in past_chains (previous blocks), no dependency is established. TX1 starts a new independent chain.

Test coverage: test_dependence_chains_1_unknown_past_handle validates this.

Verdict: ✅ Handled correctly

---

✅ 8. depth_size Computation

Scenario: Computing the correct "distance" through the dependency chain for scheduling.

How it's handled (Lines 133-142):

let mut depth_size = 0;
for dep_tx in &producer_tx {
    txs.entry(*dep_tx).and_modify(|dep_tx| {
        dep_tx.output_tx.insert(*tx_hash);
        depth_size = depth_size.max(dep_tx.depth_size + dep_tx.size);
    });
}
txs.entry(*tx_hash).and_modify(|dep_tx| {
    dep_tx.depth_size = depth_size;
});

The maximum depth is correctly propagated through producer transactions.

Test coverage: test_dependence_chains_2_local_chain_mixed_bis explicitly validates depth_size values.

Verdict: ✅ Handled correctly

---

✅ 9. Non-Allowed Handles (Handle Produced But Not Marked as Allowed)

Scenario: TX1 produces handle H but it's not marked as is_allowed, TX2 tries to consume H.

How it's handled (Lines 129-130):

for allowed_handle in &tx.allowed_handle {
    allowed_handle_tx.entry(*allowed_handle).or_insert(*tx_hash);
}

Only handles in tx.allowed_handle (which is populated only when log.is_allowed == true at line 71-73) are added to the map. Non-allowed handles cannot create dependencies.

Test coverage: test_dependence_chains_2_local_non_allowed_handle validates this.

Verdict: ✅ Handled correctly

---

✅ 10. Linear Chain Continuation

Scenario: TX1 → TX2 → TX3 in a straight line should form one chain, not three.

How it's handled: The grouping_to_chains_no_fork function (unchanged) detects when a transaction has exactly one dependency and no siblings, allowing chain continuation.

Verdict: ✅ Handled correctly (unchanged logic)

---

Minor Observations

🔸 Silent Skip in fill_tx_dependence_maps

let Some(tx) = txs.get_mut(tx_hash) else {
    error!("Tx hash {:?} not found in txs map", tx_hash);  // NEW: Added error log
    continue;
};

The PR adds an error log here (previously was silent). This is an improvement.

🔸 First-Wins vs Last-Wins Semantic Change

The old handle_creator map used insert (last producer wins), while the new code uses or_insert (first producer wins). This is correct for incremental processing because when processing in chronological order, the first transaction to produce a handle should be the canonical producer for subsequent consumers.

---

Summary Table

Edge Case	Handled?	Notes
Multiple TXs produce same handle	✅	First producer wins
Self-dependencies	✅	Skipped in scan_transactions
Out-of-order/cycles	⚠️	Intentionally not handled - trusts log order
Fork detection	✅	Unchanged logic in chain grouping
Join detection	✅	Unchanged logic in chain grouping
Cross-block dependencies	✅	Uses past_chains cache
Unknown handles	✅	Creates independent chain
depth_size computation	✅	Verified by tests
Non-allowed handles	✅	Only allowed handles create dependencies
Linear chain continuation	✅	Unchanged logic

---

Conclusion

The PR handles all edge cases correctly EXCEPT for out-of-order dependencies, which is an intentional design choice. The PR title explicitly states "trust tx & logs order from get_logs", meaning the assumption is that the upstream data source (get_logs) always returns logs in correct chronological order within a block.

Risk Assessment:

• If logs ever arrive out of order, dependencies will be silently missed (no warning logged)
• This could cause computation scheduling issues if a TX is scheduled before its dependencies
• The risk is mitigated if the data source is trusted to always provide correct ordering

Recommendation: Consider adding a debug! log when a handle lookup fails in allowed_handle_tx to help diagnose ordering issues if they ever occur in production.

---

[Eikix]

@claude /pr-review can you explain how this PR solves each potential edge case (multiple txs in the block that generate the same handle, etc.), any type of cycle. TLDR -> think of what could go wrong and look whether this PR covers every edge case.

[claude]

Claude finished @Eikix's task in 1m 39s —— View job

---

Deep Edge Case Analysis for PR #1901

[AUTOMATED]

• Gather context (PR diff and current implementation)
• Analyze all edge cases systematically
• Trace through code paths for each scenario
• Identify any gaps or uncovered edge cases
• Summarize findings

---

Overview

This PR removes the two-pass approach (tx_of_handle + topological_order) and replaces it with an incremental single-pass algorithm that processes transactions in log order, building the allowed_handle_tx map as it goes. The key assumption is that logs from get_logs are always correctly ordered chronologically.

---

Edge Case Deep Dive

✅ Edge Case 1: Multiple TXs Produce the Same Handle

Scenario: TX1 (index 0) produces handle H, TX2 (index 1) also produces handle H. TX3 (index 2) later consumes H.

Code Path (lines 128-130):

for allowed_handle in &tx.allowed_handle {
    allowed_handle_tx.entry(*allowed_handle).or_insert(*tx_hash);
}

Analysis:

• or_insert semantics: first producer wins
• When TX1 processes, H → TX1 is inserted
• When TX2 processes, entry(H) finds existing entry, or_insert does nothing
• When TX3 consumes H, it depends on TX1 (the first producer)

Why this works: The first transaction to produce a handle in chronological order is the canonical producer. This is correct because later transactions cannot have depended on TX2's production of H if TX2 came after TX1 in block order.

Test coverage: test_dependence_chains_2_local_duplicated_handle validates this exact scenario.

Verdict: ✅ Correctly handled

---

✅ Edge Case 2: Self-Dependencies (TX Uses Its Own Output)

Scenario: Within TX1, operation A produces handle H, then operation B (in same TX) consumes H.

Code Path (lines 62-66 in scan_transactions):

for input in log_inputs {
    if tx.output_handle.contains(&input) {
        // self dependency, ignore, assuming logs are ordered in tx
        continue;
    }
    tx.input_handle.push(input);
}

Analysis:

• When logs are scanned, if a handle is already in output_handle (produced earlier in same TX), it's NOT added to input_handle
• This prevents the transaction from listing itself as a dependency
• The assumption "logs are ordered in tx" means outputs appear before they're consumed within the same TX

Why this works: The filtering happens at log-scan time, so self-cycles are never even considered when building dependency maps.

Verdict: ✅ Correctly handled

---

⚠️ Edge Case 3: Out-of-Order Dependencies (Cycles)

Scenario: Due to re-org or indexer issues, TX1 appears first in logs but depends on TX2 which appears later.

OLD Behavior (deleted topological_order function):

if dep_pos > tx_pos {
    cut_deps.push(*dep);
    warn!("Cutting dependency against block order: tx {:?} depends on later tx {:?}", tx_hash, dep);
}

• Explicitly detected backward edges
• Logged a warning when cutting

NEW Behavior:

• When TX1 is processed, TX2's handle isn't in allowed_handle_tx yet
• The lookup allowed_handle_tx.get(input_handle) returns None
• TX1 simply won't establish a dependency on TX2
• No warning, no explicit handling

Key insight: The new code implicitly handles this by the nature of incremental processing:

• If TX1 comes before TX2 in log order, TX1 cannot depend on TX2 (the handle doesn't exist yet)
• The dependency is silently "not established" rather than "cut"

Difference from old code:

Aspect	Old Code	New Code
Detection	Explicit cycle detection	Implicit (handle not found)
Warning	Logged warning	Silent
Behavior	Cut the edge post-hoc	Never establish edge

Risk Assessment: If logs arrive out of order (which violates the stated assumption), dependencies are silently lost. However, the PR title explicitly states "trust tx & logs order from get_logs" - this is an intentional design choice.

Verdict: ⚠️ Intentionally not handled - relies on correct ordering assumption

---

✅ Edge Case 4: Diamond Dependencies (Fork + Join)

Scenario:

TX1 produces H1
  ├── TX2 consumes H1, produces H2
  └── TX3 consumes H1, produces H3
TX4 consumes H2 and H3 (join)

Code Path:

1. Fork detection (lines 94-110): When TX2 and TX3 both consume H1, both are recorded in used_txs_chains[TX1]
2. Join detection (in grouping_to_chains_no_fork, lines 305-306): TX4 has 2 dependencies → is_linear = false → new chain created

Analysis:

• TX4's producer_tx will contain both TX2 and TX3
• depth_size takes max of (TX2.depth_size + TX2.size, TX3.depth_size + TX3.size)
• Chain grouping correctly identifies this as non-linear → creates separate chain

Test coverage: test_dependence_chains_2_local_chain_mixed and test_dependence_chains_2_local_chain_mixed_bis

Verdict: ✅ Correctly handled

---

✅ Edge Case 5: Fork Detection (Same Handle Consumed by Multiple TXs)

Scenario: TX1 produces H1, then TX2 and TX3 both consume H1 (parallel execution).

Code Path (in grouping_to_chains_no_fork, lines 313-332):

if let Some(siblings) = used_txs_chains.get_mut(&unique_parent) {
    // ... sibling consolidation logic ...
    is_linear = siblings.len() == 1;
}

Analysis:

• When TX3 is processed, used_txs_chains[TX1] already contains TX2
• siblings.len() == 2 → is_linear = false
• TX3 starts a new chain instead of extending TX1's chain
• This prevents incorrectly merging parallel branches

Test coverage: test_past_chain_fork and test_current_block_fork

Verdict: ✅ Correctly handled

---

✅ Edge Case 6: Cross-Block Dependencies

Scenario: Handle H from block N-1 is consumed by TX1 in block N.

Code Path (lines 111-126):

} else if let Some(dep_tx_hash) = past_chains.write().await.get(input_handle) {
    // extra block, this is directly a chain hash
    tx.input_tx.insert(*dep_tx_hash);
    // ... update used_txs_chains ...
}

Analysis:

• If handle not in current block's allowed_handle_tx, check past_chains LRU cache
• If found, establish dependency on the past chain's hash
• LRU .get() also refreshes the entry (preventing eviction of hot handles)

Test coverage: test_dependence_chains_1_known_past_handle, test_dependence_chains_1_local_and_known_past_handle

Verdict: ✅ Correctly handled

---

✅ Edge Case 7: Unknown Handle (Not Produced Anywhere)

Scenario: TX1 consumes handle H that was never produced (either ancient or invalid).

Code Path:

• Not in allowed_handle_tx (current block) → first if fails
• Not in past_chains (previous blocks) → second if fails
• No dependency established
• TX1 starts its own independent chain

Test coverage: test_dependence_chains_1_unknown_past_handle

Verdict: ✅ Correctly handled

---

✅ Edge Case 8: Non-Allowed Handles

Scenario: TX1 produces handle H with is_allowed = false, TX2 tries to consume H.

Code Path (lines 71-73 in scan_transactions):

if log.is_allowed {
    tx.allowed_handle.push(output);
}

And lines 128-130:

for allowed_handle in &tx.allowed_handle {
    allowed_handle_tx.entry(*allowed_handle).or_insert(*tx_hash);
}

Analysis:

• Only handles with is_allowed = true go into allowed_handle
• Only allowed_handle entries are added to allowed_handle_tx
• Non-allowed handles cannot create intra-block dependencies
• TX2 won't find H, so no dependency established

Test coverage: test_dependence_chains_2_local_non_allowed_handle

Verdict: ✅ Correctly handled

---

✅ Edge Case 9: depth_size Computation for Scheduling

Scenario: Chain of dependencies TX1 → TX2 → TX3 needs correct scheduling priority.

Code Path (lines 132-142):

let mut depth_size = 0;
for dep_tx in &producer_tx {
    txs.entry(*dep_tx).and_modify(|dep_tx| {
        dep_tx.output_tx.insert(*tx_hash);
        depth_size = depth_size.max(dep_tx.depth_size + dep_tx.size);
    });
}
txs.entry(*tx_hash).and_modify(|dep_tx| {
    dep_tx.depth_size = depth_size;
});

Analysis:

• For each producer, compute producer.depth_size + producer.size
• Take the maximum across all producers (correct for parallel paths)
• Assign to current transaction's depth_size
• This gives the "distance from root" for scheduling priority

Test coverage: test_dependence_chains_2_local_chain_mixed_bis explicitly validates:

assert_eq!(logs[4].tx_depth_size, 2);  // TX3 depends on TX1 and TX2, each with size=1

Verdict: ✅ Correctly handled

---

✅ Edge Case 10: Linear Chain Continuation

Scenario: TX1 → TX2 → TX3 in a straight line should form one chain, not three.

Code Path (in grouping_to_chains_no_fork, lines 305-339):

• is_linear = (dependencies_block.len() + dependencies_outer.len()) == 1
• If true AND no fork (siblings check), extend existing chain

Analysis: When TX2 has exactly one dependency (TX1) and is the only child of TX1, is_linear = true and TX2 extends TX1's chain instead of creating a new one.

Verdict: ✅ Correctly handled (unchanged logic)

---

✅ Edge Case 11: Duplicated TrivialEncrypt with Same Handle

Scenario: TX1 creates H via TrivialEncrypt, TX2 re-uses the same H value via TrivialEncrypt, then uses output from TX1.

Test coverage: test_dependence_chains_duplicated_trivial_encrypt validates:

let va_1 = input_handle(&mut logs, tx1);           // H created in TX1
let vb_1 = op1(va_1, &mut logs, tx1);              // TX1 produces allowed output
let va_2 = input_shared_handle(&mut logs, va_1, tx2); // TX2 re-uses same H
let _vb_2 = op2(vb_1, va_2, &mut logs, tx2);      // TX2 consumes TX1's output
assert_eq!(chains.len(), 1);                       // Should be one chain!

Analysis:

• TX2 depends on TX1 via vb_1
• The shared handle va_1 is just a trivial encrypt, not an allowed output
• Dependency correctly established through vb_1

Verdict: ✅ Correctly handled

---

Summary: What Could Go Wrong?

Edge Case	Status	Risk Level	Notes
Multiple TXs produce same handle	✅	None	First producer wins
Self-dependencies	✅	None	Filtered at scan time
Out-of-order logs	⚠️	Medium	Silent dependency loss
Diamond dependencies	✅	None	Correct depth_size
Fork detection	✅	None	Sibling tracking
Cross-block dependencies	✅	None	LRU cache lookup
Unknown handles	✅	None	Independent chain
Non-allowed handles	✅	None	Filtered from map
depth_size computation	✅	None	Max propagation
Linear chain continuation	✅	None	Unchanged logic
Duplicated TrivialEncrypt	✅	None	Test coverage

---

The One Gap: Out-of-Order Logs

The only uncovered edge case is when logs arrive out of chronological order. The old code explicitly detected this with:

warn!("Cutting dependency against block order: tx {:?} depends on later tx {:?}", tx_hash, dep);

The new code silently doesn't establish the dependency. This is an intentional design choice as stated in the PR title: "trust tx & logs order from get_logs".

Recommendation

Consider adding a debug! or trace! log when a handle lookup fails, to help diagnose potential ordering issues in production:

if let Some(dep_tx) = allowed_handle_tx.get(input_handle) {
    // ... existing code ...
} else if let Some(dep_tx_hash) = past_chains.write().await.get(input_handle) {
    // ... existing code ...
} else {
    // New: optional diagnostic
    debug!("Handle {:?} not found in current block or past chains for tx {:?}", input_handle, tx_hash);
}

This would provide visibility without the overhead of the previous defensive cycle-cutting logic.

---

Conclusion

This PR correctly handles all edge cases under the stated assumption that logs arrive in correct chronological order.

The simplification is valid because:

1. The incremental allowed_handle_tx map naturally prevents cycles (future handles can't be referenced)
2. First-producer-wins semantic is correct for chronological processing
3. All existing tests pass, validating the behavior
4. The deleted tests (test_dependence_chains_2_local_chain_bad_tx_order, etc.) tested the now-removed defensive cycle-cutting

The only risk is silent dependency loss if logs ever arrive out of order, but this is explicitly accepted by the PR's design philosophy.

---

[Eikix]

Perhaps we can add a guard that does a local sort by (blockNumber, transactionIndex, logIndex) on txs, to harden the assumption (ethereum blocks are ordered but perhaps our RPC node not)
comparatively not too expensive?

[rudy-6-4]

Perhaps we can add a guard that does a local sort by (blockNumber, transactionIndex, logIndex) on txs, to harden the assumption (ethereum blocks are ordered but perhaps our RPC node not) comparatively not too expensive?

This is for one block only. yes if the index are available in logs, we could check the index.