Skip to content

Commit 0e511d3

Browse files
zsystmzsystm
authored
fix(ics20): correct balance accounting issue with test coverage (#219)
* tests: add ics20 precompile tests and refactor test utils - Make gas limit configurable for testing lower gas limit scenario - Get ibctesting.AppCreator when create coordinator, so other applications can leverage this test suits. - Make GenerateContractCallArgs as separate function. This doens't have to be a member method of TxFactory implementation. un-necessary dependency. - SendEvmTx now supports contract creation. * fix ibc test suite and ics20 tx vulnerability Previous ibc test suite uses different bond denom ("stake) with EVM denom ("aatom"). Even though it could be different technically, but we need to make sure everything works correctly with default config (bond denom == evm denom). Wit this commit, we can make sure IBC and ICS20 transfers with default config should work well. Updated test util for writing test cases more easily. * add revert test case * fix lint * implement safeCopyInputs and add query test case Currently, Arguments.Copy in geth panics if there are type mismatch. Returning error instead makes more sense * applied PR reviews
1 parent f7a3922 commit 0e511d3

File tree

32 files changed

+2025
-157
lines changed

32 files changed

+2025
-157
lines changed

‎contracts/solidity/precompiles/testutil/contracts/ICS20Caller.sol‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
// SPDX-License-Identifier: LGPL-3.0-only
2+
pragma solidity >=0.8.17;
3+
4+
import "../../ics20/ICS20I.sol" as ics20;
5+
import "../../common/Types.sol" as types;
6+
7+
contract ICS20Caller {
8+
int64 public counter;
9+
10+
function testIbcTransfer(
11+
string memory _sourcePort,
12+
string memory _sourceChannel,
13+
string memory _denom,
14+
uint256 _amount,
15+
address _sender,
16+
string memory _receiver,
17+
types.Height memory _timeoutHeight,
18+
uint64 _timeoutTimestamp,
19+
string memory _memo
20+
) public returns (uint64) {
21+
return
22+
ics20.ICS20_CONTRACT.transfer(
23+
_sourcePort,
24+
_sourceChannel,
25+
_denom,
26+
_amount,
27+
_sender,
28+
_receiver,
29+
_timeoutHeight,
30+
_timeoutTimestamp,
31+
_memo
32+
);
33+
}
34+
35+
function testIbcTransferFromContract(
36+
string memory _sourcePort,
37+
string memory _sourceChannel,
38+
string memory _denom,
39+
uint256 _amount,
40+
string memory _receiver,
41+
types.Height memory _timeoutHeight,
42+
uint64 _timeoutTimestamp,
43+
string memory _memo
44+
) public returns (uint64) {
45+
return
46+
ics20.ICS20_CONTRACT.transfer(
47+
_sourcePort,
48+
_sourceChannel,
49+
_denom,
50+
_amount,
51+
address(this),
52+
_receiver,
53+
_timeoutHeight,
54+
_timeoutTimestamp,
55+
_memo
56+
);
57+
}
58+
59+
function testIbcTransferWithTransfer(
60+
string memory _sourcePort,
61+
string memory _sourceChannel,
62+
string memory _denom,
63+
uint256 _amount,
64+
address _sender,
65+
string memory _receiver,
66+
types.Height memory _timeoutHeight,
67+
uint64 _timeoutTimestamp,
68+
string memory _memo,
69+
bool _before,
70+
bool _after
71+
) public returns (uint64) {
72+
if (_before) {
73+
counter++;
74+
(bool sent, ) = msg.sender.call{value: 15}("");
75+
require(sent, "Failed to send Ether to sender");
76+
}
77+
uint64 nextSequence = ics20.ICS20_CONTRACT.transfer(
78+
_sourcePort,
79+
_sourceChannel,
80+
_denom,
81+
_amount,
82+
_sender,
83+
_receiver,
84+
_timeoutHeight,
85+
_timeoutTimestamp,
86+
_memo
87+
);
88+
if (_after) {
89+
counter++;
90+
(bool sent, ) = msg.sender.call{value: 15}("");
91+
require(sent, "Failed to send Ether to sender");
92+
}
93+
return nextSequence;
94+
}
95+
96+
function testRevertIbcTransfer(
97+
string memory _sourcePort,
98+
string memory _sourceChannel,
99+
string memory _denom,
100+
uint256 _amount,
101+
address _sender,
102+
string memory _receiver,
103+
address _receiverAddr,
104+
types.Height memory _timeoutHeight,
105+
uint64 _timeoutTimestamp,
106+
string memory _memo,
107+
bool _after
108+
) public {
109+
try
110+
ICS20Caller(address(this)).ibcTransferAndRevert(
111+
_sourcePort,
112+
_sourceChannel,
113+
_denom,
114+
_amount,
115+
_sender,
116+
_receiver,
117+
_timeoutHeight,
118+
_timeoutTimestamp,
119+
_memo
120+
)
121+
{} catch {}
122+
if (_after) {
123+
counter++;
124+
(bool sent, ) = _receiverAddr.call{value: 15}("");
125+
require(sent, "Failed to send Ether to delegator");
126+
}
127+
}
128+
129+
function ibcTransferAndRevert(
130+
string memory _sourcePort,
131+
string memory _sourceChannel,
132+
string memory _denom,
133+
uint256 _amount,
134+
address _sender,
135+
string memory _receiver,
136+
types.Height memory _timeoutHeight,
137+
uint64 _timeoutTimestamp,
138+
string memory _memo
139+
) external returns (uint64 nextSequence) {
140+
nextSequence = ics20.ICS20_CONTRACT.transfer(
141+
_sourcePort,
142+
_sourceChannel,
143+
_denom,
144+
_amount,
145+
_sender,
146+
_receiver,
147+
_timeoutHeight,
148+
_timeoutTimestamp,
149+
_memo
150+
);
151+
revert();
152+
}
153+
154+
function deposit() public payable {}
155+
}

‎contracts/solidity/x/ibc/callbacks/testutil/CounterWithCallbacks.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ contract CounterWithCallbacks is ICallbacks {
101101
) external override {
102102
// Emit event when packet times out
103103
emit PacketTimedOut(channelId, portId, sequence, data);
104-
counter -= 1; // Increment counter on acknowledgement
104+
counter -= 1; // Decrement counter on timeout
105105
}
106106

107107
/**

‎evmd/precompiles.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ func NewAvailableStaticPrecompiles(
7575
}
7676

7777
ibcTransferPrecompile, err := ics20precompile.NewPrecompile(
78+
bankKeeper,
7879
stakingKeeper,
7980
transferKeeper,
8081
channelKeeper,

‎evmd/tests/ibc/helper.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type NativeErc20Info struct {
3333
}
3434

3535
// SetupNativeErc20 deploys, registers, and mints a native ERC20 token on an EVM-based chain.
36-
func SetupNativeErc20(t *testing.T, chain *evmibctesting.TestChain) *NativeErc20Info {
36+
func SetupNativeErc20(t *testing.T, chain *evmibctesting.TestChain, senderAcc evmibctesting.SenderAccount) *NativeErc20Info {
3737
t.Helper()
3838

3939
evmCtx := chain.GetContext()
@@ -65,7 +65,7 @@ func SetupNativeErc20(t *testing.T, chain *evmibctesting.TestChain) *NativeErc20
6565
contractAbi := contracts.ERC20MinterBurnerDecimalsContract.ABI
6666
nativeDenom := erc20types.CreateDenom(contractAddr.String())
6767
sendAmt := ibctesting.DefaultCoinAmount
68-
senderAcc := chain.SenderAccount.GetAddress()
68+
senderAddr := senderAcc.SenderAccount.GetAddress()
6969

7070
_, err = evmApp.EVMKeeper.CallEVM(
7171
evmCtx,
@@ -75,15 +75,15 @@ func SetupNativeErc20(t *testing.T, chain *evmibctesting.TestChain) *NativeErc20
7575
true,
7676
nil,
7777
"mint",
78-
common.BytesToAddress(senderAcc),
78+
common.BytesToAddress(senderAddr),
7979
big.NewInt(sendAmt.Int64()),
8080
)
8181
if err != nil {
8282
t.Fatalf("mint call failed: %v", err)
8383
}
8484

8585
// Verify minted balance
86-
bal := evmApp.Erc20Keeper.BalanceOf(evmCtx, contractAbi, contractAddr, common.BytesToAddress(senderAcc))
86+
bal := evmApp.Erc20Keeper.BalanceOf(evmCtx, contractAbi, contractAddr, common.BytesToAddress(senderAddr))
8787
if bal.Cmp(big.NewInt(sendAmt.Int64())) != 0 {
8888
t.Fatalf("unexpected ERC20 balance; got %s, want %s", bal.String(), sendAmt.String())
8989
}
@@ -92,7 +92,7 @@ func SetupNativeErc20(t *testing.T, chain *evmibctesting.TestChain) *NativeErc20
9292
Denom: nativeDenom,
9393
ContractAbi: contractAbi,
9494
ContractAddr: contractAddr,
95-
Account: common.BytesToAddress(senderAcc),
95+
Account: common.BytesToAddress(senderAddr),
9696
InitialBal: big.NewInt(sendAmt.Int64()),
9797
}
9898
}

0 commit comments

Comments
 (0)