Skip to content
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.

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

remove-unnecessary-events #125

Closed
wants to merge 24 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 14 additions & 0 deletions .github/workflows/abigen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,30 @@ jobs:
version: v1.61
working-directory: abigen

<<<<<<< HEAD
test:
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v4
=======
test:
runs-on: ubuntu-latest
steps:
- name: Checkout sources
uses: actions/checkout@v4
>>>>>>> df69446 (fix syntax)

- uses: actions/setup-go@v4
with:
go-version: "1.23"
cache-dependency-path: abigen/go.sum

<<<<<<< HEAD
- name: "Unit test abigen"
run: cd abigen && go test -v ./...
=======
- name: "Unit test abigen"
run: cd abigen && go test -v ./...

>>>>>>> df69446 (fix syntax)
3 changes: 3 additions & 0 deletions .github/workflows/foundry.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ on:
push:
branches:
- "main"
paths:
- "src/**"
- "test/**"

jobs:
lint:
Expand Down
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,19 @@ The following benchmarks are for a single packet transfer without aggregation.

| **Contract** | **Method** | **Description** | **Gas (groth16)** | **Gas (plonk)** |
|:---:|:---:|:---:|:---:|:---:|
<<<<<<< HEAD
| `ICS26Router.sol` | `sendPacket` | Initiating an IBC transfer with an `ERC20`. | ~205,000 | ~205,000 |
| `ICS26Router.sol` | `recvPacket` | Receiving _back_ an `ERC20` token. | ~484,000 | ~563,000 |
| `ICS26Router.sol` | `recvPacket` | Receiving a _new_ Cosmos token for the first time. (Deploying an `ERC20` contract) | ~1,422,000 | ~1,510,000 |
| `ICS26Router.sol` | `ackPacket` | Acknowledging an ICS20 packet. | ~370,000 | ~448,000 |
| `ICS26Router.sol` | `timeoutPacket` | Timing out an ICS20 packet | ~458,000 | ~545,000 |
=======
| `ICS26Router.sol` | `sendPacket` | Initiating an IBC transfer with an `ERC20`. | ~210,000 | ~210,000 |
| `ICS26Router.sol` | `recvPacket` | Receiving _back_ an `ERC20` token. | ~491,000 | ~570,000 |
| `ICS26Router.sol` | `recvPacket` | Receiving a _new_ Cosmos token for the first time. (Deploying an `ERC20` contract) | ~1,426,000 | ~1,514,000 |
| `ICS26Router.sol` | `ackPacket` | Acknowledging an ICS20 packet. | ~377,000 | ~455,000 |
| `ICS26Router.sol` | `timeoutPacket` | Timing out an ICS20 packet | ~465,000 | ~552,000 |
>>>>>>> 569bb1c (fixtures and update benchmark)

### Aggregated Packet Benchmarks

Expand All @@ -145,10 +153,15 @@ Since there is no meaningful difference in gas costs between plonk and groth16 i

| **ICS26Router Method** | **Description** | **Avg Gas (25 packets)** | **Avg Gas (50 packets)** |
|:---:|:---:|:---:|:---:|
<<<<<<< HEAD
| `multicall/recvPacket` | Receiving _back_ an `ERC20` token. | ~204,000 | ~197,000 |
| `multicall/ackPacket` | Acknowledging an ICS20 packet. | ~116,000 | ~110,000 |

Note: These gas benchmarks are with Groth16.
=======
| `multicall/recvPacket` | Receiving _back_ an `ERC20` token. | ~208,000 | ~201,000 |
| `multicall/ackPacket` | Acknowledging an ICS20 packet. | ~121,000 | ~115,000 |
>>>>>>> 569bb1c (fixtures and update benchmark)

## License

Expand Down
5 changes: 5 additions & 0 deletions e2e/interchaintestv8/chainconfig/chain_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,13 @@ var DefaultChainSpecs = []*interchaintest.ChainSpec{
ChainID: "simd-1",
Images: []ibc.DockerImage{
{
<<<<<<< HEAD
Repository: "ghcr.io/cosmos/ibc-go-wasm-simd", // FOR LOCAL IMAGE USE: Docker Image Name
Version: "gjermund-7591-ics20-abi-encoding", // FOR LOCAL IMAGE USE: Docker Image Tag
=======
Repository: "ghcr.io/cosmos/ibc-go-wasm-simd", // FOR LOCAL IMAGE USE: Docker Image Name
Version: "gjermund-abi-test", // FOR LOCAL IMAGE USE: Docker Image Tag
>>>>>>> 569bb1c (fixtures and update benchmark)
UidGid: "1025:1025",
},
},
Expand Down
8 changes: 8 additions & 0 deletions e2e/interchaintestv8/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ go 1.22.7

toolchain go1.23.2

replace github.com/cosmos/solidity-ibc-eureka/abigen => ../../abigen

require (
cosmossdk.io/api v0.7.6
cosmossdk.io/errors v1.0.1
Expand Down Expand Up @@ -304,12 +306,18 @@ require (
sigs.k8s.io/yaml v1.4.0 // indirect
)

<<<<<<< HEAD
replace github.com/cosmos/solidity-ibc-eureka/abigen => ../../abigen

replace (
github.com/cosmos/ibc-go/v9 => github.com/cosmos/ibc-go/v9 v9.0.0-20241123151201-3d84b47307b9
github.com/cosmos/ibc-go/modules/light-clients/08-wasm => github.com/cosmos/ibc-go/modules/light-clients/08-wasm v0.0.0-20241123151201-3d84b47307b9
)
=======
replace github.com/cosmos/ibc-go/v9 => github.com/cosmos/ibc-go/v9 v9.0.0-20241122125410-44814ffd0163

replace github.com/cosmos/ibc-go/modules/light-clients/08-wasm => github.com/cosmos/ibc-go/modules/light-clients/08-wasm v0.0.0-20241122125410-44814ffd0163
>>>>>>> 569bb1c (fixtures and update benchmark)

replace (
github.com/misko9/go-substrate-rpc-client/v4 => github.com/DimitrisJim/go-substrate-rpc-client/v4 v4.0.0-20240717100841-406da076c1d5
Expand Down
7 changes: 7 additions & 0 deletions e2e/interchaintestv8/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -403,10 +403,17 @@ github.com/cosmos/iavl v1.2.0 h1:kVxTmjTh4k0Dh1VNL046v6BXqKziqMDzxo93oh3kOfM=
github.com/cosmos/iavl v1.2.0/go.mod h1:HidWWLVAtODJqFD6Hbne2Y0q3SdxByJepHUOeoH4LiI=
github.com/cosmos/ibc-go/modules/capability v1.0.1 h1:ibwhrpJ3SftEEZRxCRkH0fQZ9svjthrX2+oXdZvzgGI=
github.com/cosmos/ibc-go/modules/capability v1.0.1/go.mod h1:rquyOV262nGJplkumH+/LeYs04P3eV8oB7ZM4Ygqk4E=
<<<<<<< HEAD
github.com/cosmos/ibc-go/modules/light-clients/08-wasm v0.0.0-20241123151201-3d84b47307b9 h1:xLh2Px8mToh/PbU7NzookBHLEr0h+GEnOAaaBwlF4Bc=
github.com/cosmos/ibc-go/modules/light-clients/08-wasm v0.0.0-20241123151201-3d84b47307b9/go.mod h1:mQ8AAgoyLNNjmZqjsI8sJ1+ASS0CUSDGXJNqH1SgOfI=
github.com/cosmos/ibc-go/v9 v9.0.0-20241123151201-3d84b47307b9 h1:30NCjBSKgVa4/mXj063EPBQ26ZR35EuxiSKmYXJ4sv0=
github.com/cosmos/ibc-go/v9 v9.0.0-20241123151201-3d84b47307b9/go.mod h1:TYSpg74CWeTP35UnmhqvMeFcHFp4KdTfHN3Q9IDDjtQ=
=======
github.com/cosmos/ibc-go/modules/light-clients/08-wasm v0.0.0-20241122125410-44814ffd0163 h1:U3Raxj6geXtVHJVp/IcNf5iW/Zu74IOvKVlCGwBtdGc=
github.com/cosmos/ibc-go/modules/light-clients/08-wasm v0.0.0-20241122125410-44814ffd0163/go.mod h1:mQ8AAgoyLNNjmZqjsI8sJ1+ASS0CUSDGXJNqH1SgOfI=
github.com/cosmos/ibc-go/v9 v9.0.0-20241122125410-44814ffd0163 h1:GCrIWM5VvuIRl/jb4/s1g89l2M69nM/IoHtJBADXNa0=
github.com/cosmos/ibc-go/v9 v9.0.0-20241122125410-44814ffd0163/go.mod h1:TYSpg74CWeTP35UnmhqvMeFcHFp4KdTfHN3Q9IDDjtQ=
>>>>>>> 569bb1c (fixtures and update benchmark)
github.com/cosmos/ics23/go v0.11.0 h1:jk5skjT0TqX5e5QJbEnwXIS2yI2vnmLOgpQPeM5RtnU=
github.com/cosmos/ics23/go v0.11.0/go.mod h1:A8OjxPE67hHST4Icw94hOxxFEJMBG031xIGF/JHNIY0=
github.com/cosmos/ledger-cosmos-go v0.13.3 h1:7ehuBGuyIytsXbd4MP43mLeoN2LTOEnk5nvue4rK+yM=
Expand Down
32 changes: 32 additions & 0 deletions e2e/interchaintestv8/ibc_eureka_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,11 @@ func (s *IbcEurekaTestSuite) ICS20TransferERC20TokenfromEthereumToCosmosAndBackT
s.Require().Equal(transfertypes.PortID, sendPacket.Payloads[0].DestPort)
s.Require().Equal(ibctesting.FirstChannelID, sendPacket.DestChannel)
s.Require().Equal(transfertypes.V1, sendPacket.Payloads[0].Version)
<<<<<<< HEAD
s.Require().Equal(transfertypes.EncodingABI, sendPacket.Payloads[0].Encoding)
=======
s.Require().Equal("application/x-abi", sendPacket.Payloads[0].Encoding)
>>>>>>> 569bb1c (fixtures and update benchmark)

s.True(s.Run("Verify balances on Ethereum", func() {
// User balance on Ethereum
Expand Down Expand Up @@ -541,13 +545,21 @@ func (s *IbcEurekaTestSuite) ICS20TransferERC20TokenfromEthereumToCosmosAndBackT
Receiver: strings.ToLower(ethereumUserAddress.Hex()),
Memo: "",
}
<<<<<<< HEAD
transferBz, err := ics20lib.EncodeFungibleTokenPacketData(transferPayload)
=======
transferBz, err := transfertypes.EncodeABIFungibleTokenPacketData(transferPayload)
>>>>>>> 569bb1c (fixtures and update benchmark)
s.Require().NoError(err)
payload := channeltypesv2.Payload{
SourcePort: transfertypes.PortID,
DestinationPort: transfertypes.PortID,
Version: transfertypes.V1,
<<<<<<< HEAD
Encoding: transfertypes.EncodingABI,
=======
Encoding: "application/x-abi", // TODO Get from ibc-go code when exists
>>>>>>> 569bb1c (fixtures and update benchmark)
Value: transferBz,
}

Expand All @@ -571,7 +583,11 @@ func (s *IbcEurekaTestSuite) ICS20TransferERC20TokenfromEthereumToCosmosAndBackT
// TODO: Until we get this packet from the events, we will construct it manually
// The denom should be the full denom path, not just the ibc denom
transferPayload.Denom = denomOnCosmos.Path()
<<<<<<< HEAD
payload.Value, err = ics20lib.EncodeFungibleTokenPacketData(transferPayload)
=======
payload.Value, err = transfertypes.EncodeABIFungibleTokenPacketData(transferPayload)
>>>>>>> 569bb1c (fixtures and update benchmark)
s.Require().NoError(err)
returnPacket = channeltypesv2.Packet{
Sequence: sequence,
Expand Down Expand Up @@ -759,7 +775,11 @@ func (s *IbcEurekaTestSuite) ICS20TransferNativeCosmosCoinsToEthereumAndBackTest
Receiver: strings.ToLower(ethereumUserAddress.Hex()),
Memo: sendMemo,
}
<<<<<<< HEAD
transferBz, err := ics20lib.EncodeFungibleTokenPacketData(transferPayload)
=======
transferBz, err := transfertypes.EncodeABIFungibleTokenPacketData(transferPayload)
>>>>>>> 569bb1c (fixtures and update benchmark)
s.Require().NoError(err)

payload := channeltypesv2.Payload{
Expand All @@ -786,7 +806,11 @@ func (s *IbcEurekaTestSuite) ICS20TransferNativeCosmosCoinsToEthereumAndBackTest
// TODO: Until we get this packet from the events, we will construct it manually
// The denom should be the full denom path, not just the ibc denom
transferPayload.Denom = transferCoin.Denom
<<<<<<< HEAD
payload.Value, err = ics20lib.EncodeFungibleTokenPacketData(transferPayload)
=======
payload.Value, err = transfertypes.EncodeABIFungibleTokenPacketData(transferPayload)
>>>>>>> 569bb1c (fixtures and update benchmark)
s.Require().NoError(err)
sendPacket = channeltypesv2.Packet{
Sequence: sequence,
Expand Down Expand Up @@ -845,7 +869,11 @@ func (s *IbcEurekaTestSuite) ICS20TransferNativeCosmosCoinsToEthereumAndBackTest
SourcePort: sendPacket.Payloads[0].SourcePort,
DestPort: sendPacket.Payloads[0].DestinationPort,
Version: transfertypes.V1,
<<<<<<< HEAD
Encoding: transfertypes.EncodingABI,
=======
Encoding: "application/x-abi", // TODO Get from ibc-go code when exists
>>>>>>> 569bb1c (fixtures and update benchmark)
Value: sendPacket.Payloads[0].Value,
},
},
Expand Down Expand Up @@ -983,7 +1011,11 @@ func (s *IbcEurekaTestSuite) ICS20TransferNativeCosmosCoinsToEthereumAndBackTest
s.Require().Equal(transfertypes.PortID, returnPacket.Payloads[0].DestPort)
s.Require().Equal(ibctesting.FirstChannelID, returnPacket.DestChannel)
s.Require().Equal(transfertypes.V1, returnPacket.Payloads[0].Version)
<<<<<<< HEAD
s.Require().Equal(transfertypes.EncodingABI, returnPacket.Payloads[0].Encoding)
=======
s.Require().Equal("application/x-abi", returnPacket.Payloads[0].Encoding)
>>>>>>> 569bb1c (fixtures and update benchmark)

s.True(s.Run("Verify balances on Ethereum", func() {
userBalance, err := ibcERC20.BalanceOf(nil, ethereumUserAddress)
Expand Down
12 changes: 1 addition & 11 deletions src/ICS20Transfer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,6 @@ contract ICS20Transfer is IIBCApp, IICS20Transfer, IICS20Errors, Ownable, Reentr
IBCERC20 ibcERC20Contract = IBCERC20(erc20Address);
ibcERC20Contract.burn(packetData.amount);
}

emit ICS20Transfer(packetData, erc20Address);
}

/// @inheritdoc IIBCApp
Expand Down Expand Up @@ -123,9 +121,6 @@ contract ICS20Transfer is IIBCApp, IICS20Transfer, IICS20Errors, Ownable, Reentr
// transfer the tokens to the receiver
ESCROW.send(IERC20(erc20Address), receiver, packetData.amount);

// Note the event don't take into account the conversion
emit ICS20ReceiveTransfer(packetData, erc20Address);

return ICS20Lib.SUCCESSFUL_ACKNOWLEDGEMENT_JSON;
}

Expand All @@ -139,9 +134,6 @@ contract ICS20Transfer is IIBCApp, IICS20Transfer, IICS20Errors, Ownable, Reentr
getSendERC20AddressAndSource(msg_.payload.sourcePort, msg_.sourceChannel, packetData);
_refundTokens(packetData, erc20Address);
}

// Nothing needed to be done if the acknowledgement was successful, tokens are already in escrow or burnt
emit ICS20Acknowledgement(packetData, msg_.acknowledgement);
}

/// @inheritdoc IIBCApp
Expand All @@ -150,8 +142,6 @@ contract ICS20Transfer is IIBCApp, IICS20Transfer, IICS20Errors, Ownable, Reentr
abi.decode(msg_.payload.value, (ICS20Lib.FungibleTokenPacketData));
(address erc20Address,) = getSendERC20AddressAndSource(msg_.payload.sourcePort, msg_.sourceChannel, packetData);
_refundTokens(packetData, erc20Address);

emit ICS20Timeout(packetData);
}

/// @notice Refund the tokens to the sender
Expand Down Expand Up @@ -233,7 +223,7 @@ contract ICS20Transfer is IIBCApp, IICS20Transfer, IICS20Errors, Ownable, Reentr
string calldata destChannel,
ICS20Lib.FungibleTokenPacketData memory packetData
)
private
public
returns (address, bool)
{
bytes memory denomBz = bytes(packetData.denom);
Expand Down
19 changes: 0 additions & 19 deletions src/interfaces/IICS20Transfer.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,6 @@ import { IICS20TransferMsgs } from "../msgs/IICS20TransferMsgs.sol";
import { IICS26RouterMsgs } from "../msgs/IICS26RouterMsgs.sol";

interface IICS20Transfer is IICS20TransferMsgs {
/// @notice Called when a packet is handled in onSendPacket and a transfer has been initiated
/// @param packetData The transfer packet data
/// @param erc20Address The address of the ERC20 contract of the token sent
event ICS20Transfer(ICS20Lib.FungibleTokenPacketData packetData, address erc20Address);

/// @notice Called when a packet is received in onReceivePacket
/// @param packetData The transfer packet data
/// @param erc20Address The address of the ERC20 contract of the token received
event ICS20ReceiveTransfer(ICS20Lib.FungibleTokenPacketData packetData, address erc20Address);

/// @notice Called after handling acknowledgement in onAcknowledgementPacket
/// @param packetData The transfer packet data
/// @param acknowledgement The acknowledgement data
event ICS20Acknowledgement(ICS20Lib.FungibleTokenPacketData packetData, bytes acknowledgement);

/// @notice Called after handling a timeout in onTimeoutPacket
/// @param packetData The transfer packet data
event ICS20Timeout(ICS20Lib.FungibleTokenPacketData packetData);

/// @notice Send a transfer by constructing a message and calling IICS26Router.sendPacket
/// @notice This function is not strictly necessary. You can construct IICS26RouterMsgs.SendPacketMsg
/// @notice yourself and call IICS26Router.sendPacket, which uses less gas than this function
Expand Down
Loading
Loading