feat: introduce Address newtype (closes #75)#76
Conversation
Replace the private `type Address = [u8; ADDRESS_SIZE_BYTES]` alias with a proper `Address([u8; 20])` newtype in `jet_runtime::address`. Key properties of the new type: - `#[repr(transparent)]` over `[u8; 20]` — identical memory layout, compatible with `#[repr(C)]` structs like `BlockInfo` - Derives `Clone`, `Copy`, `PartialEq`, `Eq`, `Hash`, `Default` - `Display` / `Debug` emit lowercase `0x`-prefixed hex - `FromStr` / `TryFrom<&str>` parse hex strings with optional `0x` prefix and left-pad short forms (e.g. `"0x1234"`) to 20 bytes - `From<[u8; 20]>` / `Into<[u8; 20]>` and `AsRef<[u8]>` for zero-cost interop with existing byte-slice APIs - `Address::ZERO` constant and `as_bytes_mut` for in-place mutation Updated across the codebase: - `BlockInfo::coinbase` field and getter now use `Address` - `mangle_contract_fn` takes `&Address`; `jet_contract_fn_lookup` builds an `Address` from the little-endian stack bytes (fixing an implicit reversal that previously produced inconsistent names) - `Engine::build_contract` / `run_contract` take `Address` by value - `Manager::add_contract_function` / `verify_contract` take `Address` - Test helpers and `jetdbg` construct addresses via `Address::new`, `Address::ZERO`, and `str::parse::<Address>()` Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changed Files
|
Summary of ChangesHello @tcrypt25519, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors how Ethereum addresses are handled within the runtime by introducing a dedicated Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a type-safe Address newtype, significantly improving code clarity, safety, and maintainability across the codebase by replacing raw byte arrays and string slices with a dedicated Address type. While the refactoring is clean and the new address.rs module is well-implemented, a critical regression was identified in the jet_contract_fn_lookup function. This issue, where a fixed-size array is indexed using an unconstrained iterator, can lead to a runtime panic (Denial of Service) if the input slice exceeds 20 bytes. Please address this vulnerability to ensure runtime robustness. Additionally, consider a minor optimization for the FromStr implementation for Address.
Missed in the initial pass — the test called build_contract with a bare &str; update it to pass Address::ZERO instead. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a dedicated jet_runtime::Address newtype to replace ad-hoc address representations (raw byte arrays / strings) across the runtime, compiler engine API, tests, and the jetdbg binary, aiming to centralize formatting/parsing and eliminate boilerplate.
Changes:
- Added
jet_runtime::Address([u8; 20]wrapper) with hex formatting + parsing utilities and unit tests. - Migrated contract build/run APIs and function-name mangling/lookup to use
Addressconsistently. - Updated test helpers and
jetdbgto construct and passAddressvalues instead of strings/byte vectors.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/jet_runtime/src/lib.rs | Adds address module and re-exports Address at crate root. |
| crates/jet_runtime/src/address.rs | Implements the Address newtype, formatting/parsing, and unit tests. |
| crates/jet_runtime/src/exec.rs | Replaces prior alias with Address, updates mangling and lookup path. |
| crates/jet_runtime/src/binding/mod.rs | Updates BlockInfo display to print coinbase via Address formatting. |
| crates/jet/src/engine/mod.rs | Changes Engine::build_contract / run_contract to take Address. |
| crates/jet/src/builder/manager.rs | Changes Manager::add_contract_function/verification to use Address. |
| crates/jet/tests/roms/mod.rs | Updates ROM test harness to generate/pass Address values. |
| crates/jet/src/bin/jetdbg.rs | Updates debug binary to parse and use Address values. |
Comments suppressed due to low confidence (1)
crates/jet_runtime/src/exec.rs:9
use crate::{ address::Address, ..., * }will likely fail to compile because the glob import (*) re-exportsAddressfrom the crate root (pub use address::Address), causing a duplicateAddressimport. Remove the explicitaddress::Addressimport or stop using the glob import and list required items explicitly.
use crate::{
address::Address,
error::{Result, RuntimeError},
symbols::FN_CONTRACT_PREFIX,
*,
};
…constant Three issues raised in code review: 1. jet_contract_fn_lookup: add .take(ADDRESS_SIZE_BYTES) so a 32-byte EVM stack word cannot index past the 20-byte `bytes` array and panic. The address lives in the low bytes of the word, so after reversing (little- to big-endian) we only need the first 20 bytes. 2. FromStr: replace format! + hex::decode (two heap allocations) with a stack-allocated [u8; 40] padding buffer and hex::decode_to_slice, making address parsing allocation-free. 3. Address::LEN: source from jet_ir::ADDRESS_SIZE_BYTES instead of a duplicated literal so the canonical address length is defined once. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Address::LEN = 20 is now the sole definition of address width. Every use of the literal 20 (or 40 for hex) inside the type and its impls is replaced: - struct field: [u8; Self::LEN] - ZERO constant: [0u8; Self::LEN] - new / as_bytes*: [u8; Self::LEN] - From impls: [u8; Address::LEN] - FromStr stack bufs: Address::HEX_LEN (= Self::LEN * 2, private const) - tests: Address::LEN / Address::HEX_LEN Removes the previous jet_ir::ADDRESS_SIZE_BYTES dependency; the canonical length now lives in Address itself. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
* feat: introduce Address newtype to replace raw [u8; 20] (closes #75) Replace the private `type Address = [u8; ADDRESS_SIZE_BYTES]` alias with a proper `Address([u8; 20])` newtype in `jet_runtime::address`. Key properties of the new type: - `#[repr(transparent)]` over `[u8; 20]` — identical memory layout, compatible with `#[repr(C)]` structs like `BlockInfo` - Derives `Clone`, `Copy`, `PartialEq`, `Eq`, `Hash`, `Default` - `Display` / `Debug` emit lowercase `0x`-prefixed hex - `FromStr` / `TryFrom<&str>` parse hex strings with optional `0x` prefix and left-pad short forms (e.g. `"0x1234"`) to 20 bytes - `From<[u8; 20]>` / `Into<[u8; 20]>` and `AsRef<[u8]>` for zero-cost interop with existing byte-slice APIs - `Address::ZERO` constant and `as_bytes_mut` for in-place mutation Updated across the codebase: - `BlockInfo::coinbase` field and getter now use `Address` - `mangle_contract_fn` takes `&Address`; `jet_contract_fn_lookup` builds an `Address` from the little-endian stack bytes (fixing an implicit reversal that previously produced inconsistent names) - `Engine::build_contract` / `run_contract` take `Address` by value - `Manager::add_contract_function` / `verify_contract` take `Address` - Test helpers and `jetdbg` construct addresses via `Address::new`, `Address::ZERO`, and `str::parse::<Address>()` Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * chore: CI change 🤖 * fix: update invalid_opcode test to use Address Missed in the initial pass — the test called build_contract with a bare &str; update it to pass Address::ZERO instead. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * fix: address review feedback — panic safety, alloc-free parsing, LEN constant Three issues raised in code review: 1. jet_contract_fn_lookup: add .take(ADDRESS_SIZE_BYTES) so a 32-byte EVM stack word cannot index past the 20-byte `bytes` array and panic. The address lives in the low bytes of the word, so after reversing (little- to big-endian) we only need the first 20 bytes. 2. FromStr: replace format! + hex::decode (two heap allocations) with a stack-allocated [u8; 40] padding buffer and hex::decode_to_slice, making address parsing allocation-free. 3. Address::LEN: source from jet_ir::ADDRESS_SIZE_BYTES instead of a duplicated literal so the canonical address length is defined once. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * chore: CI change 🤖 * refactor: self-referential LEN constant as single source of truth Address::LEN = 20 is now the sole definition of address width. Every use of the literal 20 (or 40 for hex) inside the type and its impls is replaced: - struct field: [u8; Self::LEN] - ZERO constant: [0u8; Self::LEN] - new / as_bytes*: [u8; Self::LEN] - From impls: [u8; Address::LEN] - FromStr stack bufs: Address::HEX_LEN (= Self::LEN * 2, private const) - tests: Address::LEN / Address::HEX_LEN Removes the previous jet_ir::ADDRESS_SIZE_BYTES dependency; the canonical length now lives in Address itself. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * chore: CI change 🤖 --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Summary
jet_runtime::Address— a#[repr(transparent)]newtype over[u8; 20]that replaces the private type alias and scattered raw byte arrays throughout the codebaseDisplay/Debug(lowercase0x-prefixed hex),FromStr/TryFrom<&str>(with left-padding for short forms like"0x1234"),From<[u8; 20]>/Into<[u8; 20]>,AsRef<[u8]>,Clone,Copy,PartialEq,Eq,Hash,DefaultAddressfromjet_runtimeat the crate rootBlockInfo::coinbase,Engine::build_contract/run_contract,Manager::add_contract_function,mangle_contract_fn,jet_contract_fn_lookup, test helpers, andjetdbgto use the new typevec![0u8; ADDRESS_SIZE_BYTES]+ manualhex::encodeat all call sitesjet_contract_fn_lookupnow produces function names consistent withmangle_contract_fn(both use the sameDisplayformat), fixing a latent name-mismatch bug in inter-contract call lookupTest plan
cargo checkpasses (verified locally)address.rscover zero address, roundtrip, full/short parse, prefix-less parse, and error casesAddressAPICloses #75
🤖 Generated with Claude Code