Skip to content

Commit

Permalink
test(contracts): improve unit and e2e gas pump tests (#1817)
Browse files Browse the repository at this point in the history
Improve unit and e2e gas pump tests.

Additional:
- fix bug in OmniGasPump.quote()
- use 10% toll

issue: #1686
  • Loading branch information
kevinhalliday authored Sep 11, 2024
1 parent c2e96c6 commit 3aa651e
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 25 deletions.
2 changes: 1 addition & 1 deletion contracts/bindings/omnigaspump.go

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions contracts/core/.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@ OmniBridgeNative_Test:test_pauseAll() (gas: 42776)
OmniBridgeNative_Test:test_pauseBridging() (gas: 36477)
OmniBridgeNative_Test:test_pauseWithdraws() (gas: 52062)
OmniBridgeNative_Test:test_withdraw() (gas: 262456)
OmniGasPump_Test:testFuzz_quote(uint32) (runs: 258, μ: 63808, ~: 63857)
OmniGasPump_Test:test_fillUp() (gas: 230967)
OmniGasPump_Test:test_pause() (gas: 62653)
OmniGasPump_Test:test_setMaxSwap() (gas: 34771)
OmniGasPump_Test:test_pause() (gas: 62701)
OmniGasPump_Test:test_setMaxSwap() (gas: 34749)
OmniGasPump_Test:test_setOmniGasStation() (gas: 36697)
OmniGasPump_Test:test_setOracle() (gas: 35014)
OmniGasPump_Test:test_setToll() (gas: 34880)
OmniGasPump_Test:test_setOracle() (gas: 34992)
OmniGasPump_Test:test_setToll() (gas: 32168)
OmniGasStation_Test:test_pause() (gas: 65220)
OmniGasStation_Test:test_setPump() (gas: 83966)
OmniGasStation_Test:test_settleUp() (gas: 365260)
Expand Down
6 changes: 4 additions & 2 deletions contracts/core/src/token/OmniGasPump.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-3.0-only
pragma solidity 0.8.24;

// solhint-disable no-console

import { OwnableUpgradeable } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
import { PausableUpgradeable } from "@openzeppelin/contracts-upgradeable/utils/PausableUpgradeable.sol";
import { XAppUpgradeable } from "src/pkg/XAppUpgradeable.sol";
Expand Down Expand Up @@ -132,7 +134,7 @@ contract OmniGasPump is XAppUpgradeable, OwnableUpgradeable, PausableUpgradeable
// take xcall fee
uint256 f = xfee();
if (amtETH < f) return (0, false, "insufficient fee");
amtETH = amtETH - f;
amtETH -= f;

// check max
if (amtETH > maxSwap) return (0, false, "over max");
Expand Down Expand Up @@ -161,7 +163,7 @@ contract OmniGasPump is XAppUpgradeable, OwnableUpgradeable, PausableUpgradeable
uint256 amtETH = _toEth(amtOMNI);

// "undo" toll
amtETH += (amtETH * TOLL_DENOM / (TOLL_DENOM - toll));
amtETH = (amtETH * TOLL_DENOM / (TOLL_DENOM - toll));

// "undo" xcall fee
return amtETH + xfee();
Expand Down
20 changes: 19 additions & 1 deletion contracts/core/test/token/OmniGasPump.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract OmniGasPump_Test is Test {
address gasStation;
address feeOracleMngr;
uint256 maxSwap = 2 ether;
uint256 toll = 50; // 5%
uint256 toll = 100; // 10%

function setUp() public {
portal = new MockPortal();
Expand Down Expand Up @@ -254,4 +254,22 @@ contract OmniGasPump_Test is Test {
vm.expectRevert("OmniGasPump: insufficient fee"); // reverts, but not becasue its paused
pump.fillUp(recipient);
}

/// @notice Test that GasPump.quote is accurate
function testFuzz_quote(uint32 _targetOMNI) public view {
uint256 targetOMNI = bound(_targetOMNI, 0.1 ether, 10 ether);
uint256 neededETH = pump.quote(targetOMNI);

(uint256 dryRunOMNI, bool wouldSucceed, string memory reason) = pump.dryFillUp(neededETH);

assertTrue(wouldSucceed, reason);

// assert quoted and actual within 10 wei of each other
// allows for rounding errors in different between taking / undoing toll
_assertWithinEpsilon(dryRunOMNI, targetOMNI, 10);
}

function _assertWithinEpsilon(uint256 a, uint256 b, uint256 epsilon) internal pure {
assertTrue(a >= b - epsilon && a <= b + epsilon);
}
}
41 changes: 30 additions & 11 deletions e2e/app/gaspump.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/omni-network/omni/lib/netconf"
"github.com/omni-network/omni/lib/txmgr"

"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/params"
Expand Down Expand Up @@ -157,23 +158,23 @@ func maybeTxHash(receipt *ethtypes.Receipt) string {
}

type GasPumpTest struct {
Recipient common.Address
AmountETH *big.Int
Recipient common.Address
TargetOMNI *big.Int
}

var (
GasPumpTests = []GasPumpTest{
{
Recipient: common.HexToAddress("0x0000000000000000000000000000000000001111"),
AmountETH: big.NewInt(5000000000000000), // 0.005 ETH
Recipient: common.HexToAddress("0x0000000000000000000000000000000000001111"),
TargetOMNI: big.NewInt(5000000000000000), // 0.005 OMNI
},
{
Recipient: common.HexToAddress("0x0000000000000000000000000000000000002222"),
AmountETH: big.NewInt(10000000000000000), // 0.01 ETH
Recipient: common.HexToAddress("0x0000000000000000000000000000000000002222"),
TargetOMNI: big.NewInt(10000000000000000), // 0.01 OMNI
},
{
Recipient: common.HexToAddress("0x0000000000000000000000000000000000003333"),
AmountETH: big.NewInt(15000000000000000), // 0.015 ETH
Recipient: common.HexToAddress("0x0000000000000000000000000000000000003333"),
TargetOMNI: big.NewInt(15000000000000000), // 0.015 OMNI
},
}
)
Expand Down Expand Up @@ -217,13 +218,31 @@ func testGasPumps(ctx context.Context, def Definition) error {
}

for _, test := range GasPumpTests {
txOpts.Value = test.AmountETH
neededETH, err := gasPump.Quote(&bind.CallOpts{Context: ctx}, test.TargetOMNI)
if err != nil {
return errors.Wrap(err, "quote", "chain", chain.Name)
}

actualOMNI, wouldSucceed, revertReason, err := gasPump.DryFillUp(&bind.CallOpts{Context: ctx}, neededETH)
if err != nil {
return errors.Wrap(err, "dry fill up", "chain", chain.Name, "needed_eth", neededETH)
}

if !wouldSucceed {
return errors.New("dry fill up failed", "chain", chain.Name, "revert_reason", revertReason)
}

if actualOMNI.Cmp(test.TargetOMNI) != 0 {
return errors.New("inaccurate quote", "chain", chain.Name, "actual_omni", actualOMNI, "provided_eth", neededETH, "target_omni", test.TargetOMNI)
}

txOpts.Value = neededETH
tx, err := gasPump.FillUp(txOpts, test.Recipient)
if err != nil {
return errors.Wrap(err, "pump", "chain", chain.Name)
return errors.Wrap(err, "pump", "chain", chain.Name, "recipient", test.Recipient.Hex(), "target_omni", test.TargetOMNI, "needed_eth", neededETH)
}

log.Info(ctx, "Pumped gas", "chain", chain.Name, "tx", tx.Hash(), "recipient", test.Recipient.Hex(), "amount", test.AmountETH)
log.Info(ctx, "Pumped gas", "chain", chain.Name, "tx", tx.Hash(), "recipient", test.Recipient.Hex(), "target_omni", test.TargetOMNI, "needed_eth", neededETH)
}
}

Expand Down
29 changes: 24 additions & 5 deletions e2e/test/gaspump_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@ package e2e_test

import (
"context"
"math/big"
"testing"

"github.com/omni-network/omni/e2e/app"
"github.com/omni-network/omni/lib/ethclient"
"github.com/omni-network/omni/lib/netconf"
"github.com/omni-network/omni/lib/xchain"

"github.com/ethereum/go-ethereum/common"

"github.com/stretchr/testify/require"
)

Expand All @@ -28,14 +31,30 @@ func TestGasPumps(t *testing.T) {
omniClient, err := ethclient.Dial(omniEVM.Name, omniRPC)
require.NoError(t, err)

// Sum targetOMNI for each chain / test case pair
// Each test case is run on per chain, except for OmniEVM

totalTargetOMNI := make(map[common.Address]*big.Int)
for _, chain := range network.EVMChains() {
// skip OmniEVM
if chain.ID == omniEVM.ID {
continue
}

for _, test := range app.GasPumpTests {
current, ok := totalTargetOMNI[test.Recipient]
if !ok {
current = big.NewInt(0)
}

totalTargetOMNI[test.Recipient] = new(big.Int).Add(current, test.TargetOMNI)
}
}

for _, test := range app.GasPumpTests {
balance, err := omniClient.BalanceAt(ctx, test.Recipient, nil)
require.NoError(t, err)

// Just test that balance > 0 for now
// TODO: assert that amount is equal to sum of AmountETH spent converted to OMNI
// Should account for the xcall fee, gas pump toll, and fee oracle conversion rates
require.Positive(t, balance.Uint64(), "recipient: %s", test.Recipient)
require.Equalf(t, totalTargetOMNI[test.Recipient], balance, "recipient: %s", test.Recipient.Hex())
}
})
}
2 changes: 1 addition & 1 deletion lib/contracts/gaspump/deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func Deploy(ctx context.Context, network netconf.ID, backend *ethbackend.Backend
GasStation: contracts.GasStation(network),
Oracle: oracle,
MaxSwap: big.NewInt(20000000000000000), // 0.02 ETH
Toll: big.NewInt(10), // 10 / 1000 = 0.1% (1000 = GasPump.TOLL_DENOM),
Toll: big.NewInt(100), // 100 / 1000 = 0.1 = 10% (1000 = GasPump.TOLL_DENOM),
ExpectedAddr: contracts.GasPump(network),
}

Expand Down

0 comments on commit 3aa651e

Please sign in to comment.