Skip to content

Multiple fixes - #2017

Open
HenriqueNogara wants to merge 7 commits into
developfrom
multiple-fixes
Open

HenriqueNogara wants to merge 7 commits into
developfrom
multiple-fixes

Conversation

@HenriqueNogara

@HenriqueNogara HenriqueNogara commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

changelog

other

  • Reject empty claims when adding/updating a compliance requirement.
  • Reject instructions underestimating the number of receipts, before they are verified
  • Add did check before freezing
  • Add destination check in transfer_funds.
  • Reject empty holder set when affirming; Cancel tasks when prunning.
  • Add new ProposalProposers storage; Prevents decoding the entire proposal when voting;
  • Reverts tokenURI if NFT was never minted

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

Several dispatch weights are stale, the migration is unbounded and undercounted, metadata snapshots are missing, and one regression test is not registered.

8 open findings
What changed in this PR

Adds validation and storage optimizations across compliance, settlement, assets, governance, and NFT precompiles.

Changes:

  • Rejects invalid compliance, affirmation, holder, and receipt inputs.
  • Adds indexed PIP proposer storage and scheduler cleanup during settlement pruning.
  • Makes ERC-721 tokenURI revert for nonexistent NFTs and expands regression coverage.
File Description
primitives/​src/​condition.rs Detects empty claim lists and adjusts complexity.
pallets/​settlement/​src/​lib.rs Adds affirmation, receipt, holder, and pruning checks.
pallets/​runtime/​tests/​src/​settlement_test.rs Tests settlement validation and pruning.
pallets/​runtime/​tests/​src/​settlement_pallet/​transfer_funds.rs Tests invalid destination portfolios.
pallets/​runtime/​tests/​src/​compliance_manager_test.rs Tests empty-claim rejection.
pallets/​runtime/​tests/​src/​asset_pallet/​mod.rs Registers freeze tests.
pallets/​runtime/​tests/​src/​asset_pallet/​freeze.rs Tests holder identity validation.
pallets/​precompiles/​src/​interface/​nft/​metadata.rs Validates NFT existence for tokenURI.
pallets/​pips/​src/​migration.rs Backfills proposal proposers.
pallets/​pips/​src/​lib.rs Adds and uses proposer index storage.
pallets/​compliance-manager/​src/​lib.rs Rejects empty claim conditions.
pallets/​asset/​src/​lib.rs Validates holders before freezing.
integration/​tests/​revive_erc721.rs Tests nonexistent NFT URI behavior.

🧠 Review effort: Balanced


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

Comment thread pallets/pips/src/lib.rs
Comment on lines +461 to +464
/// The [`Proposer`] for each proposal ([`PipId`]).
#[pallet::storage]
pub type ProposalProposers<T: Config> =
StorageMap<_, Twox64Concat, PipId, Proposer<T::AccountId>, OptionQuery>;
Comment thread pallets/pips/src/migration.rs
Comment thread pallets/asset/src/lib.rs
Comment thread pallets/pips/src/lib.rs
Comment thread pallets/pips/src/migration.rs
Comment thread pallets/runtime/tests/src/settlement_test.rs
Comment thread pallets/settlement/src/lib.rs
Comment thread pallets/settlement/src/lib.rs
@HenriqueNogara
HenriqueNogara marked this pull request as ready for review October 10, 2026 16:48
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.

2 participants