Skip to content

feat(v11): add elys xrp withdrawal #147

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AdriaCarrera merged 10 commits into from
Jun 23, 2026
Merged

feat(v11): add elys xrp withdrawal #147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a27f281
feat(v11): add elys xrp withdrawal
aluque-peersyst Jun 17, 2026
e11fcf5
fix(v11): guard elys escrow withdrawal against total-escrow underflow
aluque-peersyst Jun 18, 2026
d91d002
feat(v11): withdraw full Elys escrow per network
aluque-peersyst Jun 19, 2026
7c393a5
refactor(v11): withdraw Elys escrow via UnescrowCoin
aluque-peersyst Jun 22, 2026
ffa64c6
Merge main into feat/add-elys-icb-blocked-xrp-withdrawal
aluque-peersyst Jun 22, 2026
dd1ad24
test(poa): align AddValidator should_pass mock with delegator-form un…
aluque-peersyst Jun 23, 2026
bf9b17b
Merge origin/main into feat/add-elys-icb-blocked-xrp-withdrawal
aluque-peersyst Jun 23, 2026
c454506
Merge origin/feat/add-elys-icb-blocked-xrp-withdrawal
aluque-peersyst Jun 23, 2026
928a00b
feat(v11): added correct unescrow values for devnet, testnet, and mai…
aluque-peersyst Jun 23, 2026
0eb371d
refactor(v11): define coin by chain instead of amount
aluque-peersyst Jun 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions app/upgrades.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ func (app *App) setupUpgradeHandlers() {
app.configurator,
app.ICAHostKeeper,
app.StakingKeeper,
app.BankKeeper,
app.TransferKeeper,
),
)

Expand Down
56 changes: 55 additions & 1 deletion app/upgrades/v11/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,59 @@
package v11

import (
sdkmath "cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
)

const (
UpgradeName = "v11.0.0"
UpgradeName = "v11.0.0"
EVMCoinDenom = "axrp"

// mainnet
MainnetChainID = "xrplevm_1440000-1"
ElysChannelID = "channel-1"
WithdrawalAddress = "ethm1m2pp8zjwk3ystxyxvw5h3mrhhhnzcr2ltjntz9"

// testnet
TestnetChainID = "xrplevm_1449000-1"
TestnetElysChannelID = "channel-17"
TestnetWithdrawalAddress = "ethm16gt28px9q0fp48eatecp7j032lm5vaxs2t29pa"

// devnet
DevnetChainID = "xrplevm_1449900-1"
DevnetElysChannelID = "channel-4"
DevnetWithdrawalAddress = "ethm16gt28px9q0fp48eatecp7j032lm5vaxs2t29pa"
)

var devnetAmount, _ = sdkmath.NewIntFromString("2000000000000000000")
var testnetAmount, _ = sdkmath.NewIntFromString("2000000000000000000")
var mainnetElysAmount, _ = sdkmath.NewIntFromString("6955539034646993768414")

// ElysRecovery holds, for a single network, the Elys transfer channel whose
// escrow holds the stranded XRP, the address that should receive it, and the
// coin (denom + amount of XRP in axrp base units) to unescrow.
type ElysRecovery struct {
ChannelID string
WithdrawalAddress string
Coin sdk.Coin
}

// ElysRecoveryByNetwork maps each network's Cosmos chain ID to its Elys recovery
// parameters. The v11 handler selects the entry matching ctx.ChainID().
var ElysRecoveryByNetwork = map[string]ElysRecovery{
MainnetChainID: {
ChannelID: ElysChannelID,
WithdrawalAddress: WithdrawalAddress,
Coin: sdk.NewCoin(EVMCoinDenom, mainnetElysAmount),
},
TestnetChainID: {
ChannelID: TestnetElysChannelID,
WithdrawalAddress: TestnetWithdrawalAddress,
Coin: sdk.NewCoin(EVMCoinDenom, testnetAmount),
},
DevnetChainID: {
ChannelID: DevnetElysChannelID,
WithdrawalAddress: DevnetWithdrawalAddress,
Coin: sdk.NewCoin(EVMCoinDenom, devnetAmount),
},
}
12 changes: 12 additions & 0 deletions app/upgrades/v11/keepers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,15 @@ type StakingKeeper interface {
GetParams(ctx context.Context) (params stakingtypes.Params, err error)
SetParams(ctx context.Context, params stakingtypes.Params) error
}

// BankKeeper is the narrow interface required by the v11 upgrade
// handler. It matches a subset of bankkeeper.Keeper.
type BankKeeper interface {
GetBalance(ctx context.Context, addr sdk.AccAddress, denom string) sdk.Coin
}

// TransferKeeper is the narrow interface required by the v11 upgrade
// handler. It matches a subset of transferkeeper.Keeper.
type TransferKeeper interface {
UnescrowCoin(ctx sdk.Context, escrowAddress, receiver sdk.AccAddress, coin sdk.Coin) error
}
40 changes: 40 additions & 0 deletions app/upgrades/v11/upgrades.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,24 @@ package v11

import (
"context"
"fmt"
"time"

"cosmossdk.io/log"
upgradetypes "cosmossdk.io/x/upgrade/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
icahosttypes "github.com/cosmos/ibc-go/v10/modules/apps/27-interchain-accounts/host/types"
transfertypes "github.com/cosmos/ibc-go/v10/modules/apps/transfer/types"
)

func CreateUpgradeHandler(
mm *module.Manager,
configurator module.Configurator,
icaHostKeeper ICAHostKeeper,
stakingKeeper StakingKeeper,
bankKeeper BankKeeper,
transferKeeper TransferKeeper,
) upgradetypes.UpgradeHandler {
return func(c context.Context, _ upgradetypes.Plan, vm module.VersionMap) (module.VersionMap, error) {
ctx := sdk.UnwrapSDKContext(c)
Expand All @@ -40,7 +45,42 @@ func CreateUpgradeHandler(

logger.Info("Disabling ICA host module...")
icaHostKeeper.SetParams(ctx, icahosttypes.NewParams(false, nil))

logger.Info("Withdrawing Elys escrow to provided address...")
if err := withdrawElysEscrow(ctx, logger, bankKeeper, transferKeeper); err != nil {
return nil, err
}

logger.Info("Finished v11 upgrade handler")
return vm, nil
}
}

// withdrawElysEscrow releases the configured amount of XRP from the Elys channel
// escrow to the recovery address configured for the running network.
func withdrawElysEscrow(ctx sdk.Context, logger log.Logger, bankKeeper BankKeeper, transferKeeper TransferKeeper) error {
recoveryCfg, ok := ElysRecoveryByNetwork[ctx.ChainID()]
if !ok || recoveryCfg.ChannelID == "" {
logger.Info("no Elys escrow recovery configured for this network, skipping", "chainID", ctx.ChainID())
return nil
}

if !recoveryCfg.Coin.IsPositive() {
logger.Info("Elys escrow recovery amount is zero, nothing to withdraw", "chainID", ctx.ChainID())
return nil
}

escrowAddr := transfertypes.GetEscrowAddress(transfertypes.PortID, recoveryCfg.ChannelID)

destAddr, err := sdk.AccAddressFromBech32(recoveryCfg.WithdrawalAddress)
if err != nil {
return fmt.Errorf("invalid withdrawal address %q: %w", recoveryCfg.WithdrawalAddress, err)
}

if err := transferKeeper.UnescrowCoin(ctx, escrowAddr, destAddr, recoveryCfg.Coin); err != nil {
return fmt.Errorf("failed to unescrow %s from elys escrow: %w", recoveryCfg.Coin, err)
}
Comment on lines +61 to +82

@coderabbitai coderabbitai Bot Jun 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In cosmos/ibc-go v10.4.0, does TransferKeeper.UnescrowCoin return an error or panic when the coin amount exceeds the escrow account balance or the TotalEscrowForDenom counter?

💡 Result:

In ibc-go v10.4.0, the TransferKeeper.UnescrowCoin function does not explicitly check if the coin amount exceeds the escrow account balance or the TotalEscrowForDenom counter; instead, it relies on the underlying BankKeeper and Cosmos SDK arithmetic to handle these operations, which leads to different outcomes [1][2]. 1. Escrow Account Balance: The function calls k.BankKeeper.SendCoins to move tokens from the escrow address to the receiver [1][2]. If the escrow account has insufficient balance to cover the requested amount, the BankKeeper.SendCoins method will return an error [1]. In this case, UnescrowCoin wraps and returns that error, rather than panicking [1][2]. 2. TotalEscrowForDenom Counter: After sending the coins, the function calls currentTotalEscrow.Sub(coin) [1][2]. If the coin amount exceeds the current total escrow stored in the state, the Cosmos SDK's Int.Sub method (or similar Sub operation on the Coin type) will typically panic if it results in a negative amount [3]. While the code tracks escrow, it assumes the operation is valid based on the successful bank transfer; if the escrow state becomes inconsistent (e.g., if more tokens are unescrowed than were ever escrowed due to a malicious counterparty), this subtraction operation will trigger a panic [1][2]. The documentation explicitly notes that if a negative amount is ever attempted to be stored, the keeper function will panic [3]. Therefore, while the function handles bank-level insufficient funds via error propagation, an inconsistency between the actual escrowed tokens and the tracked TotalEscrowForDenom state will lead to a panic during the subtraction step [1][3].

Citations:

🏁 Script executed:

# First, let's look at the full function and surrounding context
cat -n app/upgrades/v11/upgrades.go | head -100

Repository: xrplevm/node

Length of output: 3734

🏁 Script executed:

# Search for ElysRecoveryByNetwork definition and bankKeeper usage
rg "ElysRecoveryByNetwork|BankKeeper" app/upgrades/v11/ -A 3 -B 1

Repository: xrplevm/node

Length of output: 2047

🏁 Script executed:

# Search for GetEVMCoinDenom implementation
rg "GetEVMCoinDenom" --type go -A 5

Repository: xrplevm/node

Length of output: 150

🏁 Script executed:

# Search for GetEVMCoinDenom - it's imported from evmtypes
rg "func.*GetEVMCoinDenom" --type go -A 5

Repository: xrplevm/node

Length of output: 150

🏁 Script executed:

# Look at the constants file to see ElysRecoveryByNetwork and related values
cat -n app/upgrades/v11/constants.go

Repository: xrplevm/node

Length of output: 2372

🏁 Script executed:

# Also check if there are any other usages of bankKeeper in the upgrade file
rg "bankKeeper\." app/upgrades/v11/

Repository: xrplevm/node

Length of output: 150

🌐 Web query:

cosmos/evm GetEVMCoinDenom what denom does it return

💡 Result:

In the Cosmos EVM module, the function GetEVMCoinDenom() returns the base denomination (a string) used as the native EVM token for the chain [1][2]. This value represents the specific token denomination from the bank module that is used for EVM state transitions, gas consumption, and other native EVM operations [3][4]. By default, this is typically set to "uatom" [5][3], but it is a configurable parameter that must match the chain's native staking and minting denominations [3][6]. It is important to distinguish this from other denominations used in the module: - GetEVMCoinDenom: Returns the base denomination (e.g., "uatom") [1][2]. - GetEVMCoinExtendedDenom: Returns the extended denomination used for 18-decimal representation if configured [2]. - GetEVMCoinDisplayDenom: Returns the human-readable display denomination (e.g., "atom") [2].

Citations:

🏁 Script executed:

# Check if there are any comments or documentation about the denom in this file
rg "denom|Denom|DENOM" app/upgrades/v11/

Repository: xrplevm/node

Length of output: 353

Remove unused bankKeeper parameter or apply balance safety cap before UnescrowCoin.

The function passes bankKeeper but never invokes GetBalance to cap the withdrawal. With a hardcoded recoveryCfg.Amount, the code relies on that value exactly matching the on-chain stranded balance:

  • If recoveryCfg.Amount exceeds the escrow account's balance for the denom, UnescrowCoin returns an error and the upgrade halts.
  • If recoveryCfg.Amount exceeds the tracked TotalEscrowForDenom, the subtraction panics during the upgrade.
  • If recoveryCfg.Amount is less than the actual stranded balance, funds remain stranded.

For mainnet, mainnetElysAmount is hardcoded to "6955539034646993768414" with a comment claiming it is the "exact XRP stranded." Confirm this value precisely matches the on-chain Elys channel escrow balance at upgrade height. If the hardcoded amount is intentional, remove the now-dead bankKeeper parameter and its interface wiring. Otherwise, re-introduce bankKeeper.GetBalance to safely cap the amount.

Also verify that evmtypes.GetEVMCoinDenom() returns the exact denomination held in the Elys channel escrow account, otherwise UnescrowCoin will operate on the wrong denom.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/upgrades/v11/upgrades.go` around lines 62 - 85, The function
`withdrawElysEscrow` passes `bankKeeper` as a parameter but never uses it, and
lacks validation to safely cap the withdrawal amount before calling
`transferKeeper.UnescrowCoin`. Either remove the unused `bankKeeper` parameter
if the hardcoded `recoveryCfg.Amount` is confirmed to exactly match the on-chain
Elys channel escrow balance, or introduce a safety cap by using
`bankKeeper.GetBalance` to retrieve the actual escrow balance and apply the
minimum of that value and `recoveryCfg.Amount` before the `UnescrowCoin` call to
prevent errors or panics. Additionally, verify that `evmtypes.GetEVMCoinDenom()`
returns the exact denomination held in the Elys channel escrow account, and
confirm the hardcoded mainnet amount matches the actual stranded balance at the
upgrade height.


logger.Info("Withdrew stranded Elys escrow", "amount", recoveryCfg.Coin.String(), "from", escrowAddr.String(), "to", destAddr.String())
return nil
}
2 changes: 1 addition & 1 deletion x/poa/keeper/msg_server_add_validator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestMsgServer_AddValidator(t *testing.T) {
stakingKeeper.EXPECT().GetAllValidators(ctx).Return([]stakingtypes.Validator{}, nil)
stakingKeeper.EXPECT().GetValidator(ctx, gomock.Any()).Return(stakingtypes.Validator{Tokens: math.NewInt(0)}, nil)
stakingKeeper.EXPECT().GetAllDelegatorDelegations(ctx, gomock.Any()).Return([]stakingtypes.Delegation{}, nil)
stakingKeeper.EXPECT().GetUnbondingDelegationsFromValidator(ctx, gomock.Any()).Return([]stakingtypes.UnbondingDelegation{}, nil)
stakingKeeper.EXPECT().GetUnbondingDelegations(ctx, gomock.Any(), gomock.Any()).Return([]stakingtypes.UnbondingDelegation{}, nil)
},
bankMocks: func(ctx sdk.Context, bankKeeper *testutil.MockBankKeeper) {
bankKeeper.EXPECT().GetBalance(ctx, gomock.Any(), gomock.Any()).Return(sdk.Coin{
Expand Down
Loading