fix: add null checks and return code validation for KECCAK256#79
Merged
Conversation
Changed Files
|
Merged
- Add null pointer check for ctx in jet_ops_keccak256 - Check return code from jet_ops_keccak256 and propagate errors via Invalid return code - Use minimal unsafe scopes with SAFETY comments per Rust 2024 edition requirements Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Consolidate multiple unsafe blocks for accessing ctx fields into a single unsafe block that dereferences ctx once, improving readability while maintaining the same safety guarantees. Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
- Clarify safety documentation for ctx parameter (valid if non-null) - Add SAFETY comment for IntValue::new call for consistency Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
- Clarify undefined behavior when result pointer is null - Expand SAFETY comment for IntValue::new to explain the contract Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix keccak256 opcode implementation to match EVM specification
fix: add null checks and return code validation for KECCAK256
Feb 15, 2026
tcrypt25519
marked this pull request as ready for review
February 15, 2026 22:00
There was a problem hiding this comment.
Pull request overview
This PR adds defensive validation and proper error propagation to the KECCAK256 implementation, ensuring robust error handling for potential null context pointers and invalid memory ranges. The changes align the KECCAK256 builtin with existing patterns used for memory expansion and other operations that interact with the execution context.
Changes:
- Added null pointer check in
jet_ops_keccak256before dereferencing the context pointer, returning -1 on null - Implemented return code checking in the caller with explicit branching to
ReturnCode::Invalidon error, following thecall_mem_expand_checkedpattern - Consolidated unsafe operations into minimal scopes with comprehensive SAFETY comments per Rust 2024 edition requirements
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/jet_runtime/src/builtins.rs | Added null pointer validation and memory range checks to jet_ops_keccak256, with minimal unsafe blocks and clear SAFETY documentation |
| crates/jet/src/builder/ops.rs | Added return code checking and error branching logic to keccak256 opcode implementation, mirroring the call_mem_expand_checked pattern |
tcrypt25519
added a commit
that referenced
this pull request
Feb 16, 2026
* fix: keccak. * fix: add null checks and return code validation for KECCAK256 (#79) * Initial plan * fix: add null check and return code validation for KECCAK256 - Add null pointer check for ctx in jet_ops_keccak256 - Check return code from jet_ops_keccak256 and propagate errors via Invalid return code - Use minimal unsafe scopes with SAFETY comments per Rust 2024 edition requirements Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * refactor: consolidate unsafe blocks in jet_ops_keccak256 Consolidate multiple unsafe blocks for accessing ctx fields into a single unsafe block that dereferences ctx once, improving readability while maintaining the same safety guarantees. Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * docs: clarify safety requirements and add SAFETY comment - Clarify safety documentation for ctx parameter (valid if non-null) - Add SAFETY comment for IntValue::new call for consistency Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * docs: improve safety documentation clarity - Clarify undefined behavior when result pointer is null - Expand SAFETY comment for IntValue::new to explain the contract Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * chore: CI change 🤖 * docs: validate u32 safety for EVM memory operations (ADR-004) (#80) * Initial plan * docs: add ADR-004 explaining why u32 is safe for EVM memory operations Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * docs: remove line numbers and clarify implemented opcodes in ADR-004 Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
Closed
tcrypt25519
added a commit
that referenced
this pull request
Feb 16, 2026
* fix: keccak. * fix: add null checks and return code validation for KECCAK256 (#79) * Initial plan * fix: add null check and return code validation for KECCAK256 - Add null pointer check for ctx in jet_ops_keccak256 - Check return code from jet_ops_keccak256 and propagate errors via Invalid return code - Use minimal unsafe scopes with SAFETY comments per Rust 2024 edition requirements Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * refactor: consolidate unsafe blocks in jet_ops_keccak256 Consolidate multiple unsafe blocks for accessing ctx fields into a single unsafe block that dereferences ctx once, improving readability while maintaining the same safety guarantees. Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * docs: clarify safety requirements and add SAFETY comment - Clarify safety documentation for ctx parameter (valid if non-null) - Add SAFETY comment for IntValue::new call for consistency Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * docs: improve safety documentation clarity - Clarify undefined behavior when result pointer is null - Expand SAFETY comment for IntValue::new to explain the contract Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * chore: CI change 🤖 * docs: validate u32 safety for EVM memory operations (ADR-004) (#80) * Initial plan * docs: add ADR-004 explaining why u32 is safe for EVM memory operations Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> * docs: remove line numbers and clarify implemented opcodes in ADR-004 Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com> --------- Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: tcrypt25519 <212655132+tcrypt25519@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses review feedback on KECCAK256 implementation to add defensive validation and proper error propagation.
Changes
jet_ops_keccak256checksctxfor null before dereferencing, returns -1 on nulljet_ops_keccak256return code and branches toReturnCode::Invalidon error, followingcall_mem_expand_checkedpatternSafety documentation clarified:
ctxmust be valid if non-null (runtime check handles null case),resultmust always be valid and non-null (UB if null).💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.