Skip to content

Meter EVM gas for asset‐ID conversion and enforce storage_growth in X-Tokens precompiles#348

Merged
lavish0000 merged 2 commits into
devfrom
fix/audit-gas-storage-issues
May 22, 2025
Merged

Meter EVM gas for asset‐ID conversion and enforce storage_growth in X-Tokens precompiles#348
lavish0000 merged 2 commits into
devfrom
fix/audit-gas-storage-issues

Conversation

@lavish0000

@lavish0000 lavish0000 commented May 22, 2025

Copy link
Copy Markdown
Member

This PR resolves two high-severity audit findings:

Assets-Factory:
• Add _handle.record_db_read::(36)?; to convert_asset_id_to_address
• Ensures the 36-byte lookup is properly charged gas

X-Tokens:
• Import pallet_balances::SYSTEM_ACCOUNT_SIZE
• Replace every try_dispatch(..., 0) with try_dispatch(..., SYSTEM_ACCOUNT_SIZE)?

audit findings 3 & 4.

@sfffaaa sfffaaa requested review from Copilot, neutrinoks and sfffaaa May 22, 2025 13:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements gas metering and enforces a minimum storage growth cost for two precompile modules.

  • Assets-Factory: meters a 36-byte DB read in convert_asset_id_to_address.
  • X-Tokens: replaces zero storage growth in try_dispatch calls with SYSTEM_ACCOUNT_SIZE.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
precompiles/assets-factory/src/lib.rs Add _handle.record_db_read::<Runtime>(36)?; to meter asset ID lookup.
precompiles/xtokens/src/lib.rs Update try_dispatch calls to use SYSTEM_ACCOUNT_SIZE instead of 0.
Comments suppressed due to low confidence (1)

precompiles/assets-factory/src/lib.rs:78

  • Add unit tests to verify that the 36-byte DB read is charged correctly and that error paths propagate as expected.
_handle.record_db_read::<Runtime>(36)?;

Comment thread precompiles/assets-factory/src/lib.rs
Comment thread precompiles/xtokens/src/lib.rs

@neutrinoks neutrinoks 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.

LGTM 🚀

@lavish0000 lavish0000 merged commit 0acb43d into dev May 22, 2025
1 check passed
@lavish0000 lavish0000 deleted the fix/audit-gas-storage-issues branch May 22, 2025 14:27
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.

4 participants