Skip to content

feat: declare precompiles using builder - #1129

Closed
dmoka wants to merge 17 commits into
masterfrom
feat/use-precompiles-builder
Closed

feat: declare precompiles using builder#1129
dmoka wants to merge 17 commits into
masterfrom
feat/use-precompiles-builder

Conversation

@dmoka

@dmoka dmoka commented Jun 12, 2025

Copy link
Copy Markdown
Contributor

TODO:

  • add missing integration tests to cover configuration
  • test in fork zombienet
  • test on 1.lark by someone else

Features:

  • use builder to declare precompiles as it is more declarative and has better checks
  • set nesting to 0 for dispatch to prevent reentrancy attack

@dmoka dmoka self-assigned this Jun 17, 2025
@github-actions

github-actions Bot commented Jun 18, 2025

Copy link
Copy Markdown

Crate versions that have been updated:

  • runtime-integration-tests: v1.52.3 -> v1.53.0
  • hydradx-runtime: v347.0.0 -> v348.0.0

Runtime version has been increased.

@dmoka
dmoka requested review from Roznovjak, enthusiastmartin and mrq1911 and removed request for mrq1911 June 27, 2025 13:02
Comment thread runtime/hydradx/src/evm/precompiles/mod.rs Outdated
@github-actions

Copy link
Copy Markdown

Quick benchmark at commit 52ca911 has been executed successfully.
View results

sender: crate::contracts::deployer(),
origin: crate::contracts::deployer(),
};
let (res, data) = Executor::<Runtime>::call(context, data, U256::zero(), 100_000);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

res here is entirely unused so maybe replace with a underscore(_)?

use primitives::constants::currency::UNITS;
use primitives::{AssetId, Balance};
use sp_core::Get;
use sp_core::{H256, U256};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need a round of cargo fmt and clippy fixes, reduce redundancies and make stuff more readable

pub fn is_oracle_address(address: H160) -> bool {
let oracle_address_prefix = &(H160::from(hex!("0000010000000000000000000000000000000000"))[0..3]);
pub fn oracle_address_prefix() -> Vec<u8> {
H160::from(hex!("0000010000000000000000000000000000000000"))[0..3].to_vec()

@talhadaar talhadaar Oct 15, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe think about evaluating this manually and keeping this as a constant somewhere in this file rather than deriving each time? In my opinion, that's always easier to maintain and much cleaner, however depending on the usage this should be meh

pub fn is_asset_address(address: H160) -> bool {
let asset_address_prefix = &(H160::from(hex!("0000000000000000000000000000000100000000"))[0..16]);
pub fn asset_address_prefix() -> Vec<u8> {
H160::from(hex!("0000000000000000000000000000000100000000"))[0..16].to_vec()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment here as above

@talhadaar talhadaar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs stylistic cleanup, clippy and fmt, otherwise LGTM

@dmoka

dmoka commented Jan 7, 2026

Copy link
Copy Markdown
Contributor Author

Closed as we discussed that the refactoring touches core layer of ethereum transaction, involving risks, with little gain.

@dmoka dmoka closed this Jan 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants