Skip to content

Improvements - #2011

Merged
Neopallium merged 10 commits into
developfrom
fixes
Sep 18, 2026
Merged

Neopallium merged 10 commits into
developfrom
fixes

Conversation

@HenriqueNogara

@HenriqueNogara HenriqueNogara commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

changelog

other

  • Revert pallet_sto::invest if settlement didn't fully execute.
  • Improve nft_redeem benchmark;
  • Sets the contract account as the current_payer in precompile calls;
  • Rejects EA nested except permissions

Comment thread pallets/nft/src/lib.rs Outdated
Comment thread pallets/precompiles/src/common.rs Outdated

Copilot AI 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.

🟡 Changes recommended

Critical and moderate issues remain in payer handling, weight accounting, and STO transactional behavior.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This pull request improves NFT redemption weighting, STO settlement validation, permission checks, and precompile fee-payer propagation.

Changes:

  • Adds worst-case NFT redemption benchmarking and weighting.
  • Rejects unsettled STO investments.
  • Rejects nested Except permissions.
  • Adds payer-context handling for precompile dispatch.

Review findings:

  • pallets/nft/src/lib.rs:336Moderate (1 vote): Preserve dynamic weighting for NFT EVM burns or update precompile gas accounting.
  • pallets/precompiles/src/common.rs:121Critical (1 vote): Preserve an existing resolved payer instead of overwriting it with the signed origin.
  • pallets/precompiles/src/common.rs:121Critical (2 votes): Account for the added payer storage accesses in wrapper weight or gas charges.
  • pallets/sto/src/lib.rs:1063Moderate (3 votes): Regenerate STO benchmark weights for the added instruction-status read.
  • pallets/sto/src/lib.rs:1063Critical (1 vote): Make the complete investment flow transactional to prevent partial state changes.
  • pallets/sto/src/lib.rs:380Nit (2 votes): Clarify the error description for instructions that were not successfully settled.
  • pallets/weights/src/pallet_nft.rs:304Critical (3 votes): Use the measured worst-case redemption weight instead of zero.
File summaries
File Summary
pallets/weights/src/pallet_nft.rs Adds NFT worst-case redemption weight implementation.
pallets/sto/src/lib.rs Validates settlement success before completing investments.
pallets/runtime/tests/src/sto_test.rs Adds mediator settlement coverage.
pallets/runtime/tests/src/nft.rs Removes obsolete redemption validation test.
pallets/runtime/tests/src/external_agents_test.rs Tests nested Except rejection.
pallets/precompiles/src/lib.rs Adds transaction-payment configuration.
pallets/precompiles/src/common.rs Adds payer context around nested dispatch.
pallets/precompiles/Cargo.toml Adds transaction-payment dependency features.
pallets/nft/src/lib.rs Uses worst-case redemption weight.
pallets/nft/src/benchmarking.rs Adds worst-case redemption benchmark.
pallets/identity/src/keys.rs Rejects nested Except permissions.
Cargo.lock Records the new dependency.
Review details

Suppressed comments (1)

pallets/nft/src/lib.rs:336

  • Common::call_runtime charges call.get_dispatch_info().call_weight, and the NFT EVM precompile still passes the collection's actual key count to avoid the max-case charge (pallets/precompiles/src/interface/nft/polymesh_specific.rs:127-141). This annotation ignores that parameter, so every EVM burn is now charged the worst-case weight (and may exceed normal gas limits once the zero weight is regenerated). Preserve a dynamic-weight path for the precompile or update the precompile and its gas accounting together.
        #[pallet::weight(<T as Config>::WeightInfo::worst_case_redeem_nft())]
  • Files reviewed: 11/12 changed files
  • Comments generated: 6
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread pallets/precompiles/src/common.rs Outdated
Comment thread pallets/precompiles/src/common.rs
Comment thread pallets/sto/src/lib.rs
Comment thread pallets/weights/src/pallet_nft.rs Outdated
Comment thread pallets/sto/src/lib.rs
Comment on lines +1063 to +1066
if !matches!(
InstructionStatuses::<T>::get(instruction_id),
InstructionStatus::Success(_)
) {
Comment thread pallets/sto/src/lib.rs Outdated
@HenriqueNogara
HenriqueNogara marked this pull request as ready for review September 18, 2026 13:57
Comment thread pallets/nft/src/lib.rs Outdated
Comment thread pallets/precompiles/src/common.rs
Co-authored-by: Copilot Autofix powered by AI <[email protected]>
@Neopallium
Neopallium enabled auto-merge (squash) September 18, 2026 17:21
@Neopallium
Neopallium merged commit d44a84c into develop Sep 18, 2026
19 checks passed
@Neopallium
Neopallium deleted the fixes branch September 18, 2026 17:41
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