fix(tokenfactory): check blocked address before minting by jhelison · Pull Request #261 · KiiChain/kiichain

jhelison · 2026-01-29T16:23:06Z

Fixes #258

Found a logic bug in mintTo() - it mints coins first, then checks if the address is blocked. When you try to mint to a blocked address, the function returns an error but the coins are already minted to the module account. They just sit there stuck.

The problem is the order of operations:

Line 22: MintCoins() runs - coins created ✓
Line 32: BlockedAddr() check - oh wait, address is blocked!
Return error - but coins already minted and stuck in module

Just moved the blocked address check before minting. Now it validates everything first, then mints only if all checks pass.

Files Changed

x/tokenfactory/keeper/bankactions.go - moved blocked address check before minting
x/tokenfactory/keeper/mintto_blocked_test.go - test case added
CHANGELOG.md - entry added

Code diff

Before:

func (k Keeper) mintTo(ctx sdk.Context, amount sdk.Coin, mintTo string) error {
    _, _, err := types.DeconstructDenom(amount.Denom)
    if err != nil {
        return err
    }

    err = k.bankKeeper.MintCoins(ctx, types.ModuleName, sdk.NewCoins(amount))  // mint first
    if err != nil {
        return err
    }

    addr, err := sdk.AccAddressFromBech32(mintTo)
    if err != nil {
        return err
    }

    if k.bankKeeper.BlockedAddr(addr) {  // check after - too late
        return fmt.Errorf("failed to mint to blocked address: %s", addr)
    }

    return k.bankKeeper.SendCoinsFromModuleToAccount(...)
}

After:

func (k Keeper) mintTo(ctx sdk.Context, amount sdk.Coin, mintTo string) error {
    _, _, err := types.DeconstructDenom(amount.Denom)
    if err != nil {
        return err
    }

    addr, err := sdk.AccAddressFromBech32(mintTo)  // validate first
    if err != nil {
        return err
    }

    if k.bankKeeper.BlockedAddr(addr) {  // check before mint
        return fmt.Errorf("failed to mint to blocked address: %s", addr)
    }

    err = k.bankKeeper.MintCoins(ctx, types.ModuleName, sdk.NewCoins(amount))  // mint after checks
    if err != nil {
        return err
    }

    return k.bankKeeper.SendCoinsFromModuleToAccount(...)
}

Test proof

Before fix:

=== RUN   TestKeeperTestSuite/TestMintToBlockedAddress
    Initial module balance: 0
    Error returned: failed to mint to blocked address
    Final module balance: 1000    ← coins stuck!
--- PASS

After fix:

=== RUN   TestKeeperTestSuite/TestMintToBlockedAddress
    Initial module balance: 0
    Error returned: failed to mint to blocked address
    Final module balance: 0    ← no coins stuck ✓
--- PASS

Fixes #258 - Move blocked address validation before MintCoins call - Prevents coins from being stuck in module account when minting to blocked addresses - Add test case to verify the fix works correctly

- Update comment to reflect fixed behavior - Replace logging with proper assertion to catch regressions

coderabbitai · 2026-01-29T16:23:39Z

Walkthrough

The changes fix a bug in the mintTo function where coins were minted before validating that the recipient address isn't blocked, potentially leaving coins stuck in the module account. The fix reorders operations to validate the address and check if it's blocked before minting. Similar validation ordering changes are applied to the burnFrom function. A new public forceTransfer method is introduced to handle token transfers with validation for blocked destinations and module accounts. A test case is added to verify that minting to blocked addresses is properly rejected. The CHANGELOG is updated to document the fix.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The changes in bankactions.go also modify burnFrom and introduce a new forceTransfer function, which are not directly required by issue `#258` and may be out of scope for the stated objective.	Clarify whether burnFrom and forceTransfer changes are necessary for fixing issue `#258`, or consider moving them to a separate PR to maintain focused scope.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: fixing the blocked address check ordering in mintTo. It directly addresses the primary objective of the PR.
Description check	✅ Passed	The description is well-detailed and directly related to the changeset. It explains the bug, the fix, and provides before/after code and test examples demonstrating the issue and solution.
Linked Issues check	✅ Passed	The PR successfully addresses issue `#258`'s requirements: it moves the blocked address check (BlockedAddr and AccAddressFromBech32) before MintCoins, adds test coverage for minting to blocked addresses, and documents the change in CHANGELOG.md.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

jhelison · 2026-01-29T16:24:52Z

This PR will complete the work from @catsmile100

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@x/tokenfactory/keeper/bankactions.go`:
- Around line 3-13: Reorder the import block in bankactions.go to satisfy gci:
group and sort imports as standard library first (fmt, sort), then external
modules (google.golang.org/grpc/codes, github.com/gogo/status), then cosmos
prefixes (sdk "github.com/cosmos/cosmos-sdk/types"), and finally local project
packages (github.com/kiichain/kiichain/v7/x/tokenfactory/types); update the
import grouping/order accordingly and run make lint-fix (or gci) to auto-apply
the correct formatting.

In `@x/tokenfactory/keeper/mintto_blocked_test.go`:
- Around line 3-7: Reorder the import block so it follows the project's gci
grouping: place any standard-library imports first (none here), then external
packages (e.g., "cosmossdk.io/math" as sdkmath and
"github.com/cosmos/cosmos-sdk/types" as sdk) on one contiguous block, add a
blank line, and then place project-local imports (e.g.,
"github.com/kiichain/kiichain/v7/x/tokenfactory/types") in their own block;
update the import ordering in mintto_blocked_test.go to match that grouping and
ensure a blank line separates the external and project-local groups.

Copilot

Pull request overview

This pull request fixes a critical logic bug in the tokenfactory module's mintTo() function where coins were being minted to the module account before checking if the recipient address was blocked, causing coins to become permanently stuck in the module when minting to blocked addresses failed.

Changes:

Reordered validation logic in mintTo() to check for blocked addresses before minting coins
Added comprehensive test case to verify the fix prevents coins from being stuck
Updated CHANGELOG.md to document the bug fix

Reviewed changes

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

File	Description
x/tokenfactory/keeper/bankactions.go	Moved blocked address validation before MintCoins operation to prevent state changes when validation fails
x/tokenfactory/keeper/mintto_blocked_test.go	Added test that verifies module balance remains unchanged when minting to blocked address fails
CHANGELOG.md	Added entry documenting the fix with reference to issue #258

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

x/tokenfactory/keeper/bankactions.go (1)
93-102: Potential bug: function fails if any module account is nil, even if unrelated to the transfer.

The loop iterates through all module accounts and returns an error if any GetModuleAccount returns nil, regardless of whether that module is relevant to the transfer. If a module account hasn't been created yet, this function will fail even for valid transfers from non-module addresses.

Additionally, the sorting adds unnecessary overhead since we're only checking for membership, not producing ordered output.
🔧 Proposed fix
-	sortedPermAddrs := make([]string, 0, len(k.permAddrs))
-	for moduleName := range k.permAddrs {
-		sortedPermAddrs = append(sortedPermAddrs, moduleName)
-	}
-	sort.Strings(sortedPermAddrs)
-
-	for _, moduleName := range sortedPermAddrs {
-		account := k.accountKeeper.GetModuleAccount(ctx, moduleName)
-		if account == nil {
-			return status.Errorf(codes.NotFound, "account %s not found", moduleName)
-		}
-
-		if account.GetAddress().Equals(fromSdkAddr) {
-			return status.Errorf(codes.Internal, "send from module acc not available")
-		}
-	}
+	for moduleName := range k.permAddrs {
+		account := k.accountKeeper.GetModuleAccount(ctx, moduleName)
+		if account != nil && account.GetAddress().Equals(fromSdkAddr) {
+			return status.Errorf(codes.Internal, "cannot force transfer from module account: %s", moduleName)
+		}
+	}

🧹 Nitpick comments (2)

x/tokenfactory/keeper/msg_server_test.go (1)
256-282: Good test coverage for the blocked address fix.

The test correctly verifies that minting to a blocked address returns an error. Consider enhancing the test to also verify that the module account balance remains unchanged after the failed mint attempt, as this was the core issue (coins getting stuck in the module account).
💡 Suggested enhancement
// Get module balance before mint attempt
moduleAddr := suite.App.AccountKeeper.GetModuleAddress("tokenfactory")
balanceBefore := suite.App.BankKeeper.GetBalance(ctx, moduleAddr, suite.defaultDenom)

// Attempt to mint to the blocked address
_, err := suite.msgServer.Mint(ctx, types.NewMsgMintTo(
    suite.TestAccs[0].String(),
    sdk.NewInt64Coin(suite.defaultDenom, 100),
    distributionModuleAddr.String(),
))

// Should fail with blocked address error
suite.Require().Error(err)
suite.Require().Contains(err.Error(), "failed to mint to blocked address")

// Verify module balance is unchanged (coins not stuck)
balanceAfter := suite.App.BankKeeper.GetBalance(ctx, moduleAddr, suite.defaultDenom)
suite.Require().Equal(balanceBefore, balanceAfter, "Module balance should remain unchanged after failed mint")
x/tokenfactory/keeper/bankactions.go (1)

96-100: Consider consistent error handling pattern.

The function mixes status.Errorf (gRPC errors) with fmt.Errorf for different error cases. If this function is called from gRPC handlers, consider using a consistent pattern for error types to ensure proper error propagation.

Also applies to: 110-110

codecov · 2026-01-29T16:56:05Z

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
x/tokenfactory/keeper/bankactions.go	0.00%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

catsmile100 added 3 commits January 28, 2026 17:05

fix(tokenfactory): check blocked address before minting

68201cc

Fixes #258 - Move blocked address validation before MintCoins call - Prevents coins from being stuck in module account when minting to blocked addresses - Add test case to verify the fix works correctly

test: update test comment and add proper assertion

910565b

- Update comment to reflect fixed behavior - Replace logging with proper assertion to catch regressions

style: run make lint-fix

0f14a27

Copilot AI review requested due to automatic review settings January 29, 2026 16:23

jhelison changed the title ~~Fix/tf blocked address~~ fix(tokenfactory): check blocked address before minting Jan 29, 2026

Copilot started reviewing on behalf of jhelison January 29, 2026 16:23 View session

coderabbitai Bot reviewed Jan 29, 2026

View reviewed changes

Comment thread x/tokenfactory/keeper/bankactions.go

Comment thread x/tokenfactory/keeper/mintto_blocked_test.go Outdated

Copilot AI reviewed Jan 29, 2026

View reviewed changes

Comment thread x/tokenfactory/keeper/bankactions.go Outdated

Comment thread x/tokenfactory/keeper/bankactions.go Outdated

Comment thread x/tokenfactory/keeper/mintto_blocked_test.go Outdated

jhelison added 2 commits January 29, 2026 13:50

feat: update mintTo with correct lint and better ordering

18beb32

test: update test for blocked addr

9d516bd

coderabbitai Bot reviewed Jan 29, 2026

View reviewed changes

jhelison mentioned this pull request Jan 29, 2026

fix(tokenfactory): check blocked address before minting #259

Closed

Thaleszh approved these changes Jan 29, 2026

View reviewed changes

jhelison merged commit a30a37f into main Jan 29, 2026
8 of 9 checks passed

jhelison deleted the fix/tf-blocked-address branch January 29, 2026 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(tokenfactory): check blocked address before minting#261

fix(tokenfactory): check blocked address before minting#261
jhelison merged 5 commits into
mainfrom
fix/tf-blocked-address

jhelison commented Jan 29, 2026

Uh oh!

coderabbitai Bot commented Jan 29, 2026 •

edited

Loading

Uh oh!

jhelison commented Jan 29, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

codecov Bot commented Jan 29, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jhelison commented Jan 29, 2026

Files Changed

Code diff

Test proof

Uh oh!

coderabbitai Bot commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Estimated code review effort

Uh oh!

jhelison commented Jan 29, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jan 29, 2026

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coderabbitai Bot commented Jan 29, 2026 •

edited

Loading