feat: convert crosschain FX to apundiai #900
Conversation
WalkthroughThis pull request introduces a comprehensive rebranding and refactoring effort focused on changing the native token denomination from "FX" to "apundiai". The changes span multiple files across the project, including configuration files, genesis state, metadata generation, and various keeper implementations. The modifications involve updating constants, renaming functions, adjusting metadata generation, and streamlining bridge token processing logic. Changes
Sequence DiagramsequenceDiagram
participant User
participant BridgeKeeper
participant ERC20Keeper
participant TokenContract
User->>BridgeKeeper: Send Bridge Token
BridgeKeeper->>BridgeKeeper: Deposit Bridge Token
BridgeKeeper->>BridgeKeeper: Swap Bridge Token
BridgeKeeper->>ERC20Keeper: Convert to Base Coin
ERC20Keeper->>TokenContract: Validate Token
TokenContract-->>ERC20Keeper: Token Validated
ERC20Keeper-->>BridgeKeeper: Base Coin
BridgeKeeper-->>User: Processed Token
Possibly related PRs
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
types/metadata_test.go (1)
9-14: Enhance test coverage with additional test cases.While the basic validation tests are good, consider adding the following test cases to ensure robust validation:
- Edge cases (empty strings, special characters)
- Invalid inputs (zero decimals, very large decimals)
- Field value assertions for the rebranded metadata (verify description, symbol, etc.)
Example test cases:
func TestNewMetadata(t *testing.T) { t.Run("valid metadata", func(t *testing.T) { metadata := NewMetadata("ABC Token", "ABC", 18) assert.NoError(t, metadata.Validate()) assert.Equal(t, "ABC Token", metadata.Name) assert.Equal(t, "ABC", metadata.Symbol) }) t.Run("default metadata after rebranding", func(t *testing.T) { metadata := NewDefaultMetadata() assert.NoError(t, metadata.Validate()) assert.Equal(t, "Pundi AIFX Token", metadata.Name) assert.Equal(t, "The native staking token of the Pundi AIFX", metadata.Description) }) t.Run("invalid inputs", func(t *testing.T) { metadata := NewMetadata("", "", 0) assert.Error(t, metadata.Validate()) }) }x/erc20/keeper/genesis.go (1)
23-27: Consider using more metadata fields for consistency.While using metadata improves consistency, consider using more fields from the metadata object:
- metadata := fxtypes.NewDefaultMetadata() - _, err := k.RegisterNativeCoin(ctx, metadata.Name, metadata.Symbol, fxtypes.DenomUnit) + metadata := fxtypes.NewDefaultMetadata() + denomUnit := metadata.DenomUnits[len(metadata.DenomUnits)-1].Exponent + _, err := k.RegisterNativeCoin(ctx, metadata.Name, metadata.Symbol, denomUnit)This change would make the code more maintainable by deriving all values from metadata.
types/constant_test.go (1)
31-37: Improve test readability and coverage.Consider these improvements to the test cases:
- Extract magic numbers into named constants
- Add test cases for different denominations
- Add negative test cases
Example:
const ( oneToken = 1 oneEther = 1e18 ) func Test_NormalizeCoin(t *testing.T) { testCases := []struct { name string input string expected sdk.Coin hasError bool }{ { name: "one token", input: fmt.Sprintf("%d%s", oneToken, DefaultDenom), expected: sdk.NewCoin(DefaultDenom, sdkmath.NewInt(oneToken)), }, { name: "one ether equivalent", input: fmt.Sprintf("%d%s", oneEther, DefaultDenom), expected: sdk.NewCoin(DefaultDenom, sdkmath.NewInt(oneEther)), }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { coin, err := sdk.ParseCoinNormalized(tc.input) if tc.hasError { require.Error(t, err) return } require.NoError(t, err) assert.Equal(t, tc.expected, coin) }) } }x/crosschain/keeper/bridge_token.go (1)
18-23: Clarify the usage of DefaultSymbol vs DefaultDenom.While the condition checks against
DefaultSymbol, the error message referencesDefaultDenom. This mixed usage might be confusing. Consider updating the error message to maintain consistency:- return types.ErrInvalid.Wrapf("%s denom decimals not match %d, expect %d", - fxtypes.DefaultDenom, claim.Decimals, fxtypes.DenomUnit) + return types.ErrInvalid.Wrapf("%s decimals not match %d, expect %d", + fxtypes.DefaultSymbol, claim.Decimals, fxtypes.DenomUnit)types/constant.go (1)
91-93: Document the swap ratio.The
SwapAmountfunction implements a fixed division by 100. This conversion rate should be documented to explain the business logic behind this specific ratio.func SwapAmount(amount sdkmath.Int) sdkmath.Int { + // Convert FX to apundiai with a 1:100 ratio return amount.QuoRaw(100) }x/crosschain/keeper/bridge_call_in.go (1)
Line range hint
72-77: Simplify the denomination check.The condition block can be simplified using early return.
- if coin.Denom == fxtypes.DefaultDenom { - continue - } - if _, err = k.erc20Keeper.BaseCoinToEvm(ctx, caller, msg.GetRefundAddr(), coin); err != nil { - return err - } + if coin.Denom != fxtypes.DefaultDenom { + if _, err = k.erc20Keeper.BaseCoinToEvm(ctx, caller, msg.GetRefundAddr(), coin); err != nil { + return err + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
app/genesis.go(1 hunks)app/genesis.json(14 hunks)local-node.sh(3 hunks)server/config/data/app.json(1 hunks)testutil/network.go(1 hunks)types/constant.go(3 hunks)types/constant_test.go(1 hunks)types/metadata.go(2 hunks)types/metadata_test.go(1 hunks)x/crosschain/keeper/bridge_call_in.go(2 hunks)x/crosschain/keeper/bridge_call_out.go(1 hunks)x/crosschain/keeper/bridge_token.go(1 hunks)x/crosschain/keeper/many_to_one.go(1 hunks)x/crosschain/keeper/msg_server_test.go(3 hunks)x/crosschain/keeper/send_to_fx.go(1 hunks)x/erc20/keeper/contract.go(1 hunks)x/erc20/keeper/erc20_token.go(1 hunks)x/erc20/keeper/genesis.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Mergify Merge Protections
- GitHub Check: Summary
🔇 Additional comments (17)
x/erc20/keeper/contract.go (1)
33-33: Verify backward compatibility with existing deployed contracts.The condition change from
DefaultDenomtoDefaultSymbolmight affect existing deployed contracts. While this aligns with the rebranding effort, we should ensure that existing contracts continue to function correctly.Let's verify other usages of DefaultDenom and DefaultSymbol:
✅ Verification successful
The change from
DefaultDenomtoDefaultSymbolis correct and consistent with the codebase.The condition is used in the context of ERC20 contract deployment where checking the token symbol is more appropriate than checking the denomination. This aligns with similar checks in the codebase, particularly in the ERC20 keeper's token handling logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for DefaultDenom and DefaultSymbol usage echo "Searching for DefaultDenom usage:" rg "DefaultDenom" -A 2 echo -e "\nSearching for DefaultSymbol usage:" rg "DefaultSymbol" -A 2Length of output: 43089
x/crosschain/keeper/send_to_fx.go (2)
39-40: LGTM! Improved variable clarity.Creating a separate variable for the amount improves code readability.
44-51: Verify error handling in the new token processing flow.The new flow with
SwapBridgeTokenand additional error handling forBridgeTokenToBaseCoinlooks good, but we should verify that all error cases are properly handled in the calling code.Let's check the implementation of these methods:
✅ Verification successful
Error handling verification completed successfully
The error handling in the token processing flow is properly implemented:
- SwapBridgeToken includes proper validation and error propagation from erc20Keeper
- BridgeTokenToBaseCoin validates input amounts and handles error cases consistently
- All calling code, including the new flow, properly checks and propagates errors
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for SwapBridgeToken and BridgeTokenToBaseCoin implementations ast-grep --pattern 'func $_(ctx $_, bridgeToken $_, amount $_) $_ { $$$ }'Length of output: 78
Script:
#!/bin/bash # Search for method implementations using ripgrep rg -A 5 "func.*SwapBridgeToken|func.*BridgeTokenToBaseCoin" --type go # Also search for any calls to these methods to understand usage patterns rg "SwapBridgeToken|BridgeTokenToBaseCoin" --type goLength of output: 3075
x/erc20/keeper/erc20_token.go (1)
17-18: Verify metadata compatibility with existing tokens.The switch from
NewFXMetaDatatoNewDefaultMetadatalooks good, but we should verify that the new metadata structure maintains compatibility with existing tokens.Let's check the implementation of the new metadata function:
✅ Verification successful
Metadata structure maintains compatibility
The new
NewDefaultMetadata()implementation maintains full compatibility with existing tokens as it:
- Uses the same
banktypes.Metadatastructure- Preserves denomination units and exponents
- Only updates branding-related content (name, description, symbol)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for NewDefaultMetadata implementation and usages ast-grep --pattern 'func NewDefaultMetadata() $_ { $$$ }' echo -e "\nChecking usages:" rg "NewDefaultMetadata" -A 2Length of output: 2124
types/constant.go (1)
19-22: Verify the token denomination changes.The rebranding from "FX" to "apundiai" includes new constants for both the new and old denominations. Ensure all references to these denominations are updated consistently across the codebase.
✅ Verification successful
Token denomination changes are properly implemented.
The rebranding from "FX" to "apundiai" is consistently implemented across the codebase. The old denomination "FX" is preserved via
FXDenomconstant for backward compatibility, while the newDefaultDenomandDefaultSymbolare properly used throughout the system.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining hardcoded "FX" strings that should use the new constants rg -l 'FX"' --type go # Search for usage of the new constants rg 'DefaultDenom|DefaultSymbol|FXDenom' --type goLength of output: 20797
x/crosschain/keeper/bridge_call_in.go (1)
33-33: Verify the impact of combining bridge token operations.The change from separate deposit and conversion calls to a single
DepositBridgeTokenToBaseCoincall simplifies the code but requires careful verification of transaction atomicity.✅ Verification successful
Bridge token operations are properly combined and atomic ✅
The implementation in
many_to_one.goshows thatDepositBridgeTokenToBaseCoinmaintains the same sequence of operations with proper error handling, ensuring transaction atomicity.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of bridge token operations to ensure consistent handling rg -A 5 'DepositBridgeToken|BridgeTokenToBaseCoin' --type goLength of output: 3721
x/crosschain/keeper/many_to_one.go (1)
17-26: Add unit tests for the new swap functionality.The new
SwapBridgeTokenfunction implements critical token conversion logic but lacks test coverage.app/genesis.go (1)
105-105: Verify metadata consistency.The change from
NewFXMetaDatatoNewDefaultMetadatashould maintain consistent metadata attributes.✅ Verification successful
Metadata implementation verified successfully
The change to
NewDefaultMetadata()maintains consistent metadata attributes across the codebase with proper denomination units, base/display names, and token details.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for metadata usage to ensure consistency rg -A 5 'NewFXMetaData|NewDefaultMetadata' --type goLength of output: 2140
Script:
#!/bin/bash # Check the complete implementation of NewDefaultMetadata ast-grep --pattern 'func NewDefaultMetadata() $_ { $$$ }'Length of output: 906
testutil/network.go (1)
104-104: LGTM! Metadata function updated for rebranding.The change from
NewFXMetadata()toNewDefaultMetadata()aligns with the rebranding effort.x/crosschain/keeper/bridge_call_out.go (1)
156-159: LGTM! Simplified bridge token processing.The refactoring improves code efficiency by combining the bridge token deposit and conversion steps into a single operation while maintaining proper error handling.
x/crosschain/keeper/msg_server_test.go (2)
17-17: LGTM! Added fxtypes import.Import added to support the use of
DefaultDenomconstant.
68-68: LGTM! Updated test error messages.Error messages now use
fxtypes.DefaultDenominstead of hardcoded values, improving maintainability.Also applies to: 186-186
local-node.sh (2)
6-6: LGTM! Added readonly variable for denomination.Good practice to use a readonly variable for the denomination, preventing accidental modifications.
17-17: LGTM! Replaced hardcoded values with variable.Consistently using
$default_denominstead of hardcoded "FX" improves maintainability.Also applies to: 43-47
app/genesis.json (3)
15-15: LGTM! Bridge configurations updated consistently.The delegate threshold denomination has been consistently updated from "FX" to "apundiai" across all bridge configurations (arbitrum, avalanche, bsc, eth, layer2, optimism, tron) while maintaining the same amount.
Also applies to: 72-72, 163-163, 243-243, 420-420, 479-479, 524-524, 605-605
202-202: LGTM! Core module configurations updated consistently.The denomination has been consistently updated from "FX" to "apundiai" across all core module configurations:
- Crisis module's constant fee
- EVM module's evm_denom
- Governance module's min_deposit
- Mint module's mint_denom
- Staking module's bond_denom
Also applies to: 281-281, 340-340, 459-459, 573-573
136-138: Verify the new display denomination configuration.A new display denomination "pundiai" with exponent 18 has been added. This is a significant change that affects how token amounts are displayed to users.
Run this script to check for any inconsistencies in denomination handling:
✅ Verification successful
Denomination configuration is correctly structured and consistent
The new display denomination "pundiai" with exponent 18 is properly configured and aligns with the base denomination "apundiai" defined in system constants. This follows the standard Cosmos SDK denomination pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent handling of the display denomination # Expected: All display-related code should handle both base and display denominations echo "Checking denomination handling..." ast-grep --pattern 'denom.*"pundiai"' ast-grep --pattern 'denom.*"apundiai"'Length of output: 157
Script:
#!/bin/bash # Broader search for denomination-related code echo "Searching for denomination references..." rg -i "pundiai" -A 3 -B 3 echo -e "\nSearching for denomination configuration files..." fd -e json -e yaml -e toml | xargs rg -l "denom" echo -e "\nSearching for denomination conversion logic..." rg -i "denom.*convert|convert.*denom" -A 3 -B 3Length of output: 66329
| Display: DefaultDenom, | ||
| Name: "Function X", | ||
| Symbol: DefaultDenom, | ||
| Display: strings.ToLower(DefaultSymbol), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent Display field handling between metadata functions.
In NewDefaultMetadata, the Display field uses strings.ToLower(DefaultSymbol), while in NewMetadata it uses the symbol directly. This inconsistency could lead to display issues.
Consider standardizing the approach:
// In NewMetadata
- Display: symbol,
+ Display: strings.ToLower(symbol),Also applies to: 43-43
| func SwapCoin(coin sdk.Coin) sdk.Coin { | ||
| if coin.Denom != FXDenom { | ||
| return coin | ||
| } | ||
| coin.Amount = SwapAmount(coin.Amount) | ||
| return coin |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for zero amounts.
The SwapCoin function should validate that the coin amount is positive before performing the swap.
func SwapCoin(coin sdk.Coin) sdk.Coin {
+ if !coin.Amount.IsPositive() {
+ return coin
+ }
if coin.Denom != FXDenom {
return coin
}
coin.Amount = SwapAmount(coin.Amount)
return coin
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func SwapCoin(coin sdk.Coin) sdk.Coin { | |
| if coin.Denom != FXDenom { | |
| return coin | |
| } | |
| coin.Amount = SwapAmount(coin.Amount) | |
| return coin | |
| func SwapCoin(coin sdk.Coin) sdk.Coin { | |
| if !coin.Amount.IsPositive() { | |
| return coin | |
| } | |
| if coin.Denom != FXDenom { | |
| return coin | |
| } | |
| coin.Amount = SwapAmount(coin.Amount) | |
| return coin | |
| } |
| if !amount.IsPositive() { | ||
| return sdk.Coin{}, err | ||
| } |
There was a problem hiding this comment.
Improve error handling in BridgeTokenToBaseCoin.
The error variable is undefined in the error return statement.
if !amount.IsPositive() {
- return sdk.Coin{}, err
+ return sdk.Coin{}, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "amount must be positive")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if !amount.IsPositive() { | |
| return sdk.Coin{}, err | |
| } | |
| if !amount.IsPositive() { | |
| return sdk.Coin{}, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "amount must be positive") | |
| } |
| @@ -66,7 +66,7 @@ | |||
| "max-txs": -1 | |||
| }, | |||
| "min-retain-blocks": 0, | |||
| "minimum-gas-prices": "4000000000000FX", | |||
| "minimum-gas-prices": "4000000000000apundiai", | |||
There was a problem hiding this comment.
💡 Codebase verification
Update gas price denomination in testnet and mainnet configurations
The gas price denomination change from "FX" to "apundiai" is incomplete. While the value (4000000000000) is consistent, the denomination needs to be updated in the following files:
public/testnet/app.toml: Currently uses "FX"public/mainnet/app.toml: Currently uses "FX"
🔗 Analysis chain
Verify gas price configuration across environments.
The minimum gas price denomination change from "FX" to "apundiai" looks correct. However, this is a critical configuration that affects transaction validation.
Run this script to check for any mismatches in gas price configurations:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any inconsistencies in gas price configurations
# Expected: All gas price configurations should use "apundiai" denomination
echo "Checking gas price configurations..."
rg -l "minimum-gas-prices.*FX"
rg -l "minimum_gas_prices.*FX"
Length of output: 196
Script:
#!/bin/bash
# Check content of files containing "FX" denomination
echo "=== Content of public/testnet/app.toml ==="
cat public/testnet/app.toml | grep -A 2 -B 2 "minimum.*FX"
echo -e "\n=== Content of public/mainnet/app.toml ==="
cat public/mainnet/app.toml | grep -A 2 -B 2 "minimum.*FX"
echo -e "\n=== Files containing new 'apundiai' denomination ==="
rg -l "apundiai"
Length of output: 1014
| @@ -111,32 +111,37 @@ | |||
| "address": "cosmos1c602zv38ht8xu8u2qcmymyl55mcyvvjrzq9ur3", | |||
| "coins": [ | |||
| { | |||
| "denom": "FX", | |||
| "denom": "apundiai", | |||
There was a problem hiding this comment.
💡 Codebase verification
Token metadata is correctly configured, but tool needs update
The bank module metadata is properly configured with the new denomination "apundiai", display name "pundiai", and correct branding (Pundi AIFX Token/PUNDIAI). However:
- Update needed in
docs/tools/balances.html: Contains hardcoded "FX" symbol check that should be updated to "PUNDIAI"
🔗 Analysis chain
Verify bank module metadata configuration.
The bank module configuration has been updated with the new denomination and metadata. The changes look good, but let's verify a few aspects:
- The display denomination "pundiai" with exponent 18 is correctly configured
- The token metadata (name, symbol) matches the rebranding requirements
Run this script to check for any inconsistencies in token metadata across the codebase:
Also applies to: 122-122, 128-144
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any inconsistencies in token metadata
# Expected: All references should use updated names and symbols
echo "Checking token metadata consistency..."
rg -l "Function X"
rg -l "FX Token"
rg -l 'symbol.*"FX"'
Length of output: 393
Script:
#!/bin/bash
# Check the complete token metadata configuration and context of references
echo "=== Checking genesis.json token metadata ==="
cat app/genesis.json | jq '.app_state.bank.denom_metadata'
echo -e "\n=== Checking context of Function X references ==="
rg "Function X" -C 2
echo -e "\n=== Checking context of FX Token references ==="
rg "FX Token" -C 2
echo -e "\n=== Checking symbol FX references ==="
rg 'symbol.*"FX"' -C 2
Length of output: 20985
Summary by CodeRabbit
Release Notes
New Features
Improvements
Branding