Skip to content

fix(grc20reg): prevent token path overwrite and aliases#5907

Merged
moul merged 7 commits into
gnolang:masterfrom
notJoon:fix/grc20-register
Jul 16, 2026
Merged

fix(grc20reg): prevent token path overwrite and aliases#5907
moul merged 7 commits into
gnolang:masterfrom
notJoon:fix/grc20-register

Conversation

@notJoon

@notJoon notJoon commented Jul 7, 2026

Copy link
Copy Markdown
Member

Description

Related to gnoswap-labs/gnoswap#1334

This PR makes grc20reg.Register use canonical token registrations.

Previously, registry keys were built from the caller provided slug as the registry key and wrote with registry.Set, allowing existing token paths to be overwritten and the same token identity to be registered under multiple aliases.

The new behavior:

  • derives registry keys from the registering realm path and token symbol, matching token.ID()
  • rejects nil tokens and tokens from a different realm
  • prevents overwriting existing registry keys
  • prevents alias-style alternate lookup keys
  • emits canonical token_path and symbol in register events

Tests and atomicswap examples were updated to use canonical token paths.

@github-actions github-actions Bot added the 🧾 package/realm Tag used for new Realms or Packages. label Jul 7, 2026
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Jul 7, 2026
@Gno2D2

Gno2D2 commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):
  • IGNORE the bot requirements for this PR (force green CI check)
Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

🟢 Maintainers must be able to edit this pull request (more info)
🟢 Pending initial approval by a review team member, or review from tech-staff

☑️ Contributor Actions:
  1. Fix any issues flagged by automated checks.
  2. Follow the Contributor Checklist to ensure your PR is ready for review.
    • Add new tests, or document why they are unnecessary.
    • Provide clear examples/screenshots, if necessary.
    • Update documentation, if required.
    • Ensure no breaking changes, or include BREAKING CHANGE notes.
    • Link related issues/PRs, where applicable.
☑️ Reviewer Actions:
  1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.
📚 Resources:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: notJoon/gno-core)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🟢 Requirement satisfied
└── 🟢 If
    ├── 🟢 Condition
    │   └── 🟢 Or
    │       ├── 🟢 User davd-gzl already reviewed PR 5907 with state APPROVED
    │       ├── 🟢 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🟢 Then
        └── 🟢 Not (🔴 This label is applied to pull request: review/triage-pending)

Manual Checks
**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

  • Any user with comment edit permission

@github-actions github-actions Bot added the 📦 ⛰️ gno.land Issues or PRs gno.land package related label Jul 14, 2026
@lbrown2007
lbrown2007 requested a review from jefft0 July 15, 2026 15:25

@davd-gzl davd-gzl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be rebased on #5908.

Comment thread examples/gno.land/r/demo/defi/grc20reg/grc20reg.gno
Comment thread examples/gno.land/r/demo/defi/grc20reg/grc20reg.gno Outdated
@Gno2D2 Gno2D2 removed the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Jul 15, 2026
@jefft0
jefft0 removed their request for review July 15, 2026 17:33
@moul

moul commented Jul 16, 2026

Copy link
Copy Markdown
Member

Review note — MERGE-READY. The fix is sound: Register keys off token.ID() (bound unforgeably at grc20.NewToken under IsCurrent()), enforces key == token.ID(), and blocks overwrite via registry.Has(key) and aliasing by removing slug from key construction. Auth model correct (crossing fn, cur.Previous().PkgPath()), determinism fine (avl.Tree). Good negative tests.

Two non-blocking warnings:

  1. slug is now effectively dead — validated and emitted in the register event but never influences the key or storage. Callers still pass slugs that are silently ignored for lookup, and the event carries a slug attr that no longer maps to anything retrievable. Consider deprecating/removing it or documenting that it's cosmetic.
  2. Breaking key-format change for tokens previously registered with an empty slug (bare pkgPathpkgPath.SYMBOL). In-repo consumers are all migrated; any off-repo/on-chain Get(pkgPath) breaks. Worth calling out as intentional in the description.

⚠️ Cross-PR interaction with #5908: that PR changes Token.ID()'s format (adds a seqid component). Since this PR's registry key derives from token.ID(), merge order matters — whichever lands second needs a rebase and a re-check that keys still resolve.

🤖 Posted autonomously by an AI review agent on @moul's behalf; not human-reviewed before posting.

@moul
moul enabled auto-merge (squash) July 16, 2026 13:39
@moul
moul merged commit 33323a2 into gnolang:master Jul 16, 2026
83 checks passed
@github-project-automation github-project-automation Bot moved this from In Review to Done in 🤝🏻 Partner: Onbloc Jul 16, 2026
moul added a commit to notJoon/gno-core that referenced this pull request Jul 16, 2026
After gnolang#5907 merged, consumers (atomicswap, treasury) that look tokens up by rlmPath.symbol broke because gnolang#5908 had switched the registry key to Token.ID() (rlmPath.symbol.<seqid>). Keep the registry keyed by rlmPath.symbol (one token per realm+symbol, preserving gnolang#5907's overwrite/alias guards); Token.ID() keeps its trailing sequence id for event/identity uniqueness only. Register now returns the key; tokenhub and tests migrated off Token.ID()-based lookups.
@notJoon
notJoon deleted the fix/grc20-register branch July 20, 2026 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📦 ⛰️ gno.land Issues or PRs gno.land package related 🧾 package/realm Tag used for new Realms or Packages.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants