Skip to content

Evm precompile maintenance skill - #2998

Draft
UnArbosFour wants to merge 5 commits into
mainfrom
feat/add-evm-maintainer-skill
Draft

Evm precompile maintenance skill#2998
UnArbosFour wants to merge 5 commits into
mainfrom
feat/add-evm-maintainer-skill

Conversation

@UnArbosFour

Copy link
Copy Markdown
Contributor

Adds public documentation for current and proposed EVM precompiles, including ABI lifecycle and subscription-based event reporting. Extends the EVM maintainer skill with focused guidance for ABI versioning, backwards compatibility, coverage, and regression testing—requiring migration before any mainnet change that would force hard deprecation.

@vercel

vercel Bot commented Jul 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
subtensor Ready Ready Preview Jul 29, 2026 10:22pm

Request Review

@UnArbosFour UnArbosFour added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Jul 27, 2026
@github-actions

github-actions Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: VULNERABLE

VERY HIGH scrutiny: 27-day-old account with 1 public repository and 32 lifetime contributions, despite repository write permission; no Gittensor association or author/committer mismatch found. Branch targets main.

Static analysis found no runtime, dependency, workflow, credential, or .github/ai-review trust-boundary changes. The previously reported unsafe balance-conversion instruction remains.

Findings

Sev File Finding
HIGH .agents/skills/emv-maintainer/SKILL.md:95 Do not apply 18-decimal scaling to every precompile balance inline

Prior-comment reconciliation

  • 94f81df7: not addressed — The blanket 10^9 conversion instruction remains at lines 94–95.

Conclusion

The maintainer skill can still direct future precompile work to change established economic values by a factor of 10⁹. Balance units must be determined and preserved per released ABI.


📜 Previous run (superseded)
Sev File Finding Status
HIGH .agents/skills/emv-maintainer/SKILL.md:84 Do not apply 18-decimal scaling to every precompile balance ➡️ Carried forward to current findings
The blanket 10^9 conversion instruction remains at lines 94–95.

🔍 AI Review — Auditor (domain review)

VERDICT: 👎

LIKELY Gittensor-associated contributor; newer account with repository write access, so executable maintainer guidance received close correctness scrutiny.

The Auditor proposed a replacement PR description, but the current body is non-trivial; not overwriting. Maintainers: ask the Auditor to regenerate if you want it.

This documentation-only PR does not affect the runtime or require a spec_version bump. No overlapping PRs were identified, and no build was needed for the static findings below.

Findings

Sev File Finding
HIGH .agents/skills/emv-maintainer/SKILL.md:63 Do not apply 18-decimal scaling to every precompile balance inline
MEDIUM docs/guides/evm/precompiles/staking-v2.mdx:29 Document units for positional staking amounts inline

Conclusion

The blanket balance-scaling instruction can cause future precompile implementations to alter economic values by 10⁹. Correct the unit guidance and document V2 units before merging.

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +62 to +63
## Step 1 - Review current precompiles vs. subtensor functionality

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.

[HIGH] Do not apply 18-decimal scaling to every precompile balance

This contradicts the deployed Staking V2 contract: explicit amount_rao and amount_alpha parameters are passed to the pallet in native units without BalanceConverter. Following this instruction would multiply or divide economic values by 10⁹ and could break new functions or adapters. Scaling must follow each released function's unit contract, not a blanket rule.

Suggested change
## Step 1 - Review current precompiles vs. subtensor functionality
- Preserve each released function's documented unit contract. Use `BalanceConverter`
only for interfaces defined in 18-decimal EVM units; pass parameters defined
in native rao or alpha units through without decimal scaling.

transferStake(bytes32,bytes32,uint256,uint256,uint256)
burnAlpha(bytes32,uint256,uint256)
```

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.

[MEDIUM] Document units for positional staking amounts

This public reference lists several indistinguishable uint256 arguments without names or units. Their meanings differ materially: for example, addStake accepts rao while removeStake accepts alpha. Add parameter names and explicit units for every amount and price argument so callers do not apply the V1 18-decimal convention to V2.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread .agents/skills/emv-maintainer/SKILL.md Outdated
Comment on lines +63 to +64

- All extrinsics should be exposed to precompile callers for the following pallets:

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.

[HIGH] Do not apply 18-decimal scaling to every precompile balance

This blanket rule conflicts with deployed interfaces such as BalancePrecompile::getFreeBalance, which returns the underlying Rao value directly. Following it would multiply some established results by 10⁹ or divide inputs that already use runtime units, breaking ABI semantics and potentially causing severe economic miscalculations. Require preservation of each released function's existing unit convention, and use the EVM balance converter only where that function's ABI calls for 18-decimal values.

Suggested change
- All extrinsics should be exposed to precompile callers for the following pallets:
- Preserve each released function's documented units, scaling, precision, and rounding.
Use the runtime EVM balance converter only where that function's ABI requires 18-decimal values.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +55 to +56
- Multiply Subtensor balances by `10^9` to match EVM's 18-decimal convention,
and divide by the same factor before passing balances to Subtensor pallets.

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.

[HIGH] Do not apply 18-decimal scaling to every precompile balance

This blanket rule remains unsafe: released precompiles can define different units, and applying 10^9 universally can change economic values by that factor or double-convert an existing interface. Require each function to preserve its documented ABI units and make any conversion explicit and tested.

Suggested change
- Multiply Subtensor balances by `10^9` to match EVM's 18-decimal convention,
and divide by the same factor before passing balances to Subtensor pallets.
- Preserve each released function's documented balance units, precision, scaling,
and rounding. For new interfaces, define and test conversions per function; do not assume every balance uses 18-decimal EVM units.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +83 to +84
precompile callers for the following pallets:
- subtensor

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.

[HIGH] Do not apply 18-decimal scaling to every precompile balance

This blanket rule conflicts with released interfaces that explicitly use rao (for example, staking parameters named amount_rao) and with the repository documentation stating that precompile amounts are rao rather than 18-decimal wei. Following it would multiply returned values or divide inputs by 10^9, breaking existing contracts and potentially changing transferred/staked economic amounts. Require unit handling to follow each released ABI; only apply scaling where that specific interface already defines 18-decimal values.

Suggested change
precompile callers for the following pallets:
- subtensor
- Preserve the units, precision, scaling, and rounding defined by each released ABI.
Do not apply a blanket decimal conversion: use 18-decimal scaling only where
that specific interface already requires it, and preserve rao-denominated fields.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot 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.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +94 to +95
precompile callers for the following pallets:
- subtensor

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.

[HIGH] Do not apply 18-decimal scaling to every precompile balance

This blanket rule conflicts with the skill's compatibility requirements and existing released ABIs: for example, IBalance.getFreeBalance and staking calls explicitly expose amounts in rao, while the runtime's 10^9 converter applies to native EVM account balances. Following this instruction would multiply existing precompile results or divide their inputs by 10^9, breaking callers and potentially changing transferred or staked value. Require each function to preserve its released units; use scaling only where that function's established ABI explicitly requires 18-decimal EVM units.

Suggested change
precompile callers for the following pallets:
- subtensor
- Preserve the units and scaling defined by each released precompile ABI; several
existing interfaces expose balances directly in rao. Apply `10^9` conversion
only where the specific interface explicitly uses 18-decimal EVM units.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant