TF20 - CW20 wrapper for TokenFactory denoms

[ash-burnt]

No description provided.

[reecepbcups]

LGTM upon 2nd review

[crucible-burnt]

🔍 Crucible Security Review

Summary

Adds TF20 - a CW20 wrapper contract for TokenFactory denoms. Provides a standard CW20 interface with token admin functions (mint, burn, force transfer) backed by TokenFactory.

Security Assessment

• Risk Level: Low (standard token wrapper pattern)
• Admin gating: Proper assert_admin checks on all privileged operations ✓
• Address validation: All recipient/owner addresses validated before use
• No re-entrancy: TokenFactory messages are properly constructed and dispatched
• Mint/Burn/ForceTransfer: All properly authenticated and validated
• Admin state: Requires admin to be set (fails open if None) ✓

Implementation Quality:

• Correct use of cosmwasm_std and cosmos_sdk_proto
• Proper AnyMsg construction for TokenFactory messages
• Clean separation of concerns

Immunefi Pattern Check

• ✅ No unvalidated external calls
• ✅ No state mutation without authentication
• ✅ Admin checks properly enforced
• ✅ No integer overflow concerns (using Uint128)

False Report Risk

• None identified - straightforward token wrapper

Code Quality

• Good error handling with custom error types
• Clear function organization
• Proper CW20 compliance

Recommendation

Approve - Well-implemented token wrapper with proper admin controls and validation.

[crucible-burnt]

🔍 Crucible Security Review

Summary

CW20-compatible wrapper contract for x/tokenfactory denoms. Provides CW20 standard interface (transfer, allowance, send) over TokenFactory tokens with admin controls for minting, force transfers, and burns.

Security Assessment

• Risk Level: High (handles token operations with admin force capabilities)

Findings:

1. Admin authorization is properly enforced (src/contract.rs:130-141): assert_admin() correctly checks against stored admin and fails if admin is None. All admin functions (Mint, ForceTransfer, ForceBurn, ForceSend, UpdateContractAdmin, UpdateTokenFactoryAdmin, ModifyMetadata) call assert_admin().

2. ⚠️ send_from uses info.sender instead of owner as transfer source (src/contract.rs:397): In send_from, after deducting allowance from owner, the _send call passes info.sender.into_string() as the sender instead of owner. This means the tokens are transferred FROM the spender, not the owner. This appears to be a bug — should be owner to match CW20 semantics where send_from transfers tokens from the owner's balance using the spender's allowance.

3. _transfer and _burn use MsgForceTransfer/MsgBurn from TokenFactory: These are admin-level TF operations. The contract must be the TF denom admin for these to work. This is documented in instantiation comments.

4. UpdateContractAdmin with empty string removes admin: Setting new_admin to empty makes admin None, permanently disabling admin functions. This is documented behavior but irreversible — consider requiring explicit confirmation or a separate RemoveAdmin message.

5. No set_contract_version in migrate: There's no migrate entry point. If contract upgrades are needed, one should be added with proper version tracking.

6. Protobuf serialization via to_json_binary for AnyMsg.value: The MsgMint, MsgForceTransfer, MsgBurn, etc. are serialized with to_json_binary. Verify that the chain accepts JSON-encoded protobuf messages in AnyMsg — some chains require binary protobuf encoding. If this hasn't been tested on-chain, this could silently fail.

7. No zero-amount check: Transfer, send, burn operations don't check for amount == 0. While TokenFactory may reject zero amounts, explicit validation provides clearer error messages.

8. Allowance deduction ordering is correct: deduct_allowance is called before _transfer/_send/_burn in all *_from functions — prevents state manipulation.

Immunefi Pattern Check

• ForceTransfer/ForceBurn are powerful admin operations — admin key compromise would allow arbitrary token theft
• No JWT or signature validation involved
• TokenFactory integration means security depends on proper TF admin setup

False Report Risk

• The ForceTransfer and ForceBurn functions could attract reports claiming "admin can steal funds" — this is by design for TokenFactory admin functionality. Consider documenting this explicitly.
• The _transfer function using MsgForceTransfer for ALL transfers (not just admin-forced ones) might look suspicious but is required because the contract holds TF admin rights and tokens aren't held by the contract.

Code Quality Notes

• Well-structured with clear separation of admin vs user functions
• CW20 allowance integration uses well-tested cw20-base library
• Missing: unit tests, integration tests, event emissions for admin operations
• amount.clone() on Uint128 is unnecessary (it's Copy) — minor style issue

Status

⚠️ Potential bug in send_from (item #2) should be investigated — it passes info.sender instead of owner to _send, which would transfer tokens from the wrong address. This is the most critical finding. The rest of the contract follows sound patterns.