solana/cs_token_pool : fallback to erc20 decimals for pools without getTokenDecimals#21698
solana/cs_token_pool : fallback to erc20 decimals for pools without getTokenDecimals#216980xsuryansh wants to merge 1 commit intodevelopfrom
Conversation
|
✅ No conflicts with other open PRs targeting |
0xsuryansh
requested review from
AmrMohamedRezk,
agusaldasoro,
asoliman92,
b-gopalswami,
makramkd,
ogtownsend and
winder
|
0xsuryansh
changed the title
0xsuryansh
changed the title
There was a problem hiding this comment.
Pull request overview
Risk Rating: MEDIUM
This PR updates the Solana v0.1.1 token-pool changeset logic to determine EVM token decimals more robustly by falling back to querying the underlying ERC20 token when the pool’s getTokenDecimals call fails.
Changes:
- Introduces an ERC20-based fallback for fetching decimals when
evmTokenPool.GetTokenDecimals(...)errors. - Reuses a single
bind.CallOptsinstance for the EVM calls withingetOnChainEVMPoolConfig. - Adds an ERC20 wrapper dependency to support the fallback decimals query.
| tokenAddr, err2 := evmTokenPool.GetToken(callOpts) | ||
| if err2 != nil { | ||
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token address from evm token pool: %w", err2) | ||
| } | ||
| token, err2 := erc20.NewERC20(tokenAddr, evmChain.Client) | ||
| if err2 != nil { | ||
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to bind erc20 to fetch decimals at %s: %w", tokenAddr.Hex(), err2) | ||
| } |
There was a problem hiding this comment.
The fallback path re-reads the token address from the pool via evmTokenPool.GetToken(...), but evmTokenAddress was already fetched earlier by GetTokenStateFromPoolEVM. Consider reusing evmTokenAddress here to avoid an extra RPC call and keep the value consistent with the earlier lookup.
| tokenAddr, err2 := evmTokenPool.GetToken(callOpts) | |
| if err2 != nil { | |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token address from evm token pool: %w", err2) | |
| } | |
| token, err2 := erc20.NewERC20(tokenAddr, evmChain.Client) | |
| if err2 != nil { | |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to bind erc20 to fetch decimals at %s: %w", tokenAddr.Hex(), err2) | |
| } | |
| token, err2 := erc20.NewERC20(evmTokenAddress, evmChain.Client) | |
| if err2 != nil { | |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to bind erc20 to fetch decimals at %s: %w", evmTokenAddress.Hex(), err2) | |
| } |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token address from evm token pool: %w", err2) | ||
| } | ||
| token, err2 := erc20.NewERC20(tokenAddr, evmChain.Client) | ||
| if err2 != nil { | ||
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to bind erc20 to fetch decimals at %s: %w", tokenAddr.Hex(), err2) | ||
| } | ||
| evmTokenDecimals, err2 = token.Decimals(callOpts) | ||
| if err2 != nil { | ||
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token decimals from token contract: %w", err2) |
There was a problem hiding this comment.
If the fallback fails (e.g., GetToken/Decimals errors), the returned error drops the original GetTokenDecimals failure that triggered the fallback. Please include/wrap the original err in fallback error messages so the root cause of the pool call failure is still visible when debugging.
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token address from evm token pool: %w", err2) | |
| } | |
| token, err2 := erc20.NewERC20(tokenAddr, evmChain.Client) | |
| if err2 != nil { | |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to bind erc20 to fetch decimals at %s: %w", tokenAddr.Hex(), err2) | |
| } | |
| evmTokenDecimals, err2 = token.Decimals(callOpts) | |
| if err2 != nil { | |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token decimals from token contract: %w", err2) | |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token address from evm token pool (original GetTokenDecimals error: %v): %w", err, err2) | |
| } | |
| token, err2 := erc20.NewERC20(tokenAddr, evmChain.Client) | |
| if err2 != nil { | |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to bind erc20 to fetch decimals at %s (original GetTokenDecimals error: %v): %w", tokenAddr.Hex(), err, err2) | |
| } | |
| evmTokenDecimals, err2 = token.Decimals(callOpts) | |
| if err2 != nil { | |
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token decimals from token contract (original GetTokenDecimals error: %v): %w", err, err2) |
| evmTokenDecimals, err := evmTokenPool.GetTokenDecimals(callOpts) | ||
| if err != nil { | ||
| return solBaseTokenPool.RemoteConfig{}, fmt.Errorf("failed to get token decimals: %w", err) | ||
| // Fallback: some pool ABIs omit GetTokenDecimals; read decimals from the underlying ERC20. | ||
| tokenAddr, err2 := evmTokenPool.GetToken(callOpts) |
There was a problem hiding this comment.
This change adds a new fallback behavior when GetTokenDecimals fails, but the existing Solana v0.1.1 changeset tests appear to only cover the happy path where GetTokenDecimals succeeds. Please add a test that exercises the fallback branch (e.g., a pool contract instance that lacks getTokenDecimals/reverts on that selector) and asserts decimals are read from the ERC20 token.
No description provided.