Check if remainder is empty in event parsing.

[kubaplas]

No description provided.

[github-actions]

Benchmark report

Action	Details	Gas diff
Wasm Deploy	Filename: Benchmark.wasm	🔴 +0.005587935 CSPR (0.00%)

[llamapreview]

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Request Changes

This PR introduces a breaking change by enforcing strict remainder validation in event parsing, which could silently fail existing custom hooks that rely on non-empty remainders.

Findings Summary

Priority	File	Category	Impact Summary	Anchors
P1	core/src/host.rs	Architecture	Breaking change for FromBytes impls with non-empty remainders	FromBytes
P2	core/src/host.rs	Testing	New error case untested across execution environments	odra_vm_host, livenet_host
P2	core/src/host.rs	Maintainability	Ambiguous error term needs documentation clarification
P2	core/src/host.rs	Architecture	Error handling consistency should be verified	EventError, Formatting

🔍 Notable Themes

• The change enforces stricter data integrity but introduces a breaking change that may disrupt existing event parsing workflows.
• Error handling ambiguities and insufficient testing could lead to inconsistent behavior across different execution environments.

📈 Risk Diagram

This diagram illustrates the risk of breaking changes in event deserialization due to strict remainder validation.

sequenceDiagram
    participant C as Caller
    participant H as Host
    C->>H: get_event()
    H->>H: Parse event bytes
    note over H: R1(P1): Strict remainder validation<br/>may break existing FromBytes impls
    alt Remainder is empty
        H-->>C: Ok(event)
    else Remainder not empty
        H-->>C: Err(EventError::Formatting)
    end

Loading

---

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

[llamapreview]

P1 | Confidence: High

• The change introduces strict remainder validation for event deserialization, which improves data integrity. However, this creates a breaking change for any FromBytes implementations that currently return non-empty remainders. The related_context shows multiple FromBytes implementations (Address, PublicKey, and macro-generated ones) that return remainders from nested deserializations. If any event types use these implementations and expect partial consumption of the byte stream, they will now fail with EventError::Formatting. This change effectively enforces that events must be fully self-contained data structures.
• Speculative: The change introduces a new error case (EventError::Formatting) that should be tested across different execution environments. The related_context shows multiple host implementations (odra-vm, odra-casper) that implement the same interface. While the core logic changed, there's no evidence of corresponding test updates to verify the new error case propagates correctly through these different host implementations. This could lead to inconsistent error handling across execution environments.
• The error mapping from non-empty remainder to EventError::Formatting could benefit from clearer documentation. The term "Formatting" might be ambiguous - it could mean either malformed data or unexpected extra data. Adding a brief comment explaining that this error indicates "unconsumed bytes remain after deserialization" would improve maintainability and help distinguish it from other parsing errors.
• Speculative: The error handling strategy should be verified for consistency. The code maps all FromBytes errors to EventError::Parsing, but now introduces EventError::Formatting for non-empty remainders. This creates two distinct error cases that might overlap semantically. If the EventError enum doesn't already have clear documentation distinguishing these cases, it could lead to confusion in error handling by callers.

Suggested change

	let (event, remainder) = T::from_bytes(&bytes)
	.map_err(|_| EventError::Parsing)?;
	if remainder.is_empty() {
	Ok(event)
	} else {
	Err(EventError::Formatting)
	if remainder.is_empty() {
	Ok(event)
	} else {
	// Event data contains unconsumed bytes after deserialization
	Err(EventError::Formatting)
	}

Evidence: symbol:FromBytes, method:from_bytes, path:odra-vm/src/odra_vm_host.rs, path:odra-casper/livenet-env/src/livenet_host.rs, symbol:EventError, constant:Formatting