Skip to content

Commit

Permalink
EIP-1559: Be resilient to target gas used 0 and cap gas used to gas l…
Browse files Browse the repository at this point in the history
…imit (#1885)

* eip-1559 fixes

* fix

* add test for capping gas used

* fix linter

* try to fix one insufficient fee

* comment out other integration tests

* Revert "comment out other integration tests"

This reverts commit ac0a0e6.

* fix insufficient fee issue

* fix insufficient fee issue

* fix syntax issue
  • Loading branch information
jewei1997 authored Oct 23, 2024
1 parent a45b5ca commit 81c0d5f
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 27 deletions.
18 changes: 9 additions & 9 deletions contracts/test/ERC20toCW20PointerTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ describe("ERC20 to CW20 Pointer", function () {
});

describe("transfer()", function () {
it.only("should transfer", async function () {
it("should transfer", async function () {
let sender = accounts[0];
let recipient = accounts[1];

Expand Down Expand Up @@ -194,7 +194,7 @@ describe("ERC20 to CW20 Pointer", function () {
it("should lower approval", async function () {
const owner = accounts[0].evmAddress;
const spender = accounts[1].evmAddress;
const tx = await pointer.approve(spender, 0);
const tx = await pointer.approve(spender, 0, { gasPrice: ethers.parseUnits('100', 'gwei') });
await tx.wait();
const allowance = await pointer.allowance(owner, spender);
expect(Number(allowance)).to.equal(0);
Expand All @@ -204,21 +204,21 @@ describe("ERC20 to CW20 Pointer", function () {
const owner = accounts[0].evmAddress;
const spender = accounts[1].evmAddress;
const maxUint128 = new BigNumber("0xffffffffffffffffffffffffffffffff", 16);
const tx = await pointer.approve(spender, maxUint128.toFixed());
const tx = await pointer.approve(spender, maxUint128.toFixed(), { gasPrice: ethers.parseUnits('100', 'gwei') });
await tx.wait();
const allowance = await pointer.allowance(owner, spender);
expect(allowance).to.equal(maxUint128.toFixed());

// approving uint128 max int + 1 should work but only approve uint128
const maxUint128Plus1 = maxUint128.plus(1);
const tx128plus1 = await pointer.approve(spender, maxUint128Plus1.toFixed());
const tx128plus1 = await pointer.approve(spender, maxUint128Plus1.toFixed(), { gasPrice: ethers.parseUnits('100', 'gwei') });
await tx128plus1.wait();
const allowance128plus1 = await pointer.allowance(owner, spender);
expect(allowance128plus1).to.equal(maxUint128.toFixed());

// approving uint256 should also work but only approve uint128
const maxUint256 = new BigNumber("0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff", 16);
const tx256 = await pointer.approve(spender, maxUint256.toFixed());
const tx256 = await pointer.approve(spender, maxUint256.toFixed(), { gasPrice: ethers.parseUnits('100', 'gwei') });
await tx256.wait();
const allowance256 = await pointer.allowance(owner, spender);
expect(allowance256).to.equal(maxUint128.toFixed());
Expand All @@ -244,7 +244,7 @@ describe("ERC20 to CW20 Pointer", function () {
expect(Number(allowanceBefore)).to.be.greaterThanOrEqual(amountToTransfer);

// transfer
const tfTx = await pointer.connect(spender.signer).transferFrom(owner.evmAddress, recipient.evmAddress, amountToTransfer);
const tfTx = await pointer.connect(spender.signer).transferFrom(owner.evmAddress, recipient.evmAddress, amountToTransfer, { gasPrice: ethers.parseUnits('100', 'gwei') });
const receipt = await tfTx.wait();

// capture balances after
Expand Down Expand Up @@ -280,12 +280,12 @@ describe("ERC20 to CW20 Pointer", function () {
const owner = accounts[0];
const spender = accounts[1];

const tx = await pointer.approve(spender.evmAddress, 10);
const tx = await pointer.approve(spender.evmAddress, 10, { gasPrice: ethers.parseUnits('100', 'gwei') });
await tx.wait();

await expect(pointer.connect(spender.signer).transferFrom(owner.evmAddress, recipient.evmAddress, 20)).to.be.revertedWith("CosmWasm execute failed");
await expect(pointer.connect(spender.signer).transferFrom(owner.evmAddress, recipient.evmAddress, 20, { gasPrice: ethers.parseUnits('100', 'gwei') })).to.be.revertedWith("CosmWasm execute failed");
// put it back
await (await pointer.approve(spender.evmAddress, 0)).wait()
await (await pointer.approve(spender.evmAddress, 0, { gasPrice: ethers.parseUnits('100', 'gwei') })).wait()
});
});
});
Expand Down
1 change: 0 additions & 1 deletion loadtest/contracts/evm/bindings/erc20/erc20.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 9 additions & 1 deletion x/evm/keeper/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,18 @@ func (k *Keeper) AdjustDynamicBaseFeePerGas(ctx sdk.Context, blockGasUsed uint64
return nil
}
currentBaseFee := k.GetDynamicBaseFeePerGas(ctx)
targetGasUsed := sdk.NewDec(int64(k.GetTargetGasUsedPerBlock(ctx)))
if targetGasUsed.IsZero() {
return &currentBaseFee
}
minimumFeePerGas := k.GetParams(ctx).MinimumFeePerGas
blockGasLimit := sdk.NewDec(ctx.ConsensusParams().Block.MaxGas)
blockGasUsedDec := sdk.NewDec(int64(blockGasUsed))
targetGasUsed := sdk.NewDec(int64(k.GetTargetGasUsedPerBlock(ctx)))

// cap block gas used to block gas limit
if blockGasUsedDec.GT(blockGasLimit) {
blockGasUsedDec = blockGasLimit
}

var newBaseFee sdk.Dec
if blockGasUsedDec.GT(targetGasUsed) {
Expand Down
23 changes: 23 additions & 0 deletions x/evm/keeper/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,29 @@ func TestAdjustBaseFeePerGas(t *testing.T) {
targetGasUsed: 500000,
expectedBaseFee: 99, // Should not go below the minimum fee
},
{
name: "target gas used is 0",
currentBaseFee: 10000,
minimumFee: 10,
blockGasUsed: 0,
blockGasLimit: 1000000,
upwardAdj: sdk.NewDecWithPrec(5, 1),
downwardAdj: sdk.NewDecWithPrec(5, 1),
targetGasUsed: 0,
expectedBaseFee: 10000,
},
{
name: "cap block gas used to block gas limit",
// block gas used is 1.5x block gas limit
currentBaseFee: 10000,
minimumFee: 10,
blockGasUsed: 1500000,
blockGasLimit: 1000000,
upwardAdj: sdk.NewDecWithPrec(5, 1),
downwardAdj: sdk.NewDecWithPrec(5, 1),
targetGasUsed: 500000,
expectedBaseFee: 15000,
},
}

for _, tc := range testCases {
Expand Down
16 changes: 1 addition & 15 deletions x/evm/types/params.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (p *Params) ParamSetPairs() paramtypes.ParamSetPairs {
paramtypes.NewParamSetPair(KeyMinFeePerGas, &p.MinimumFeePerGas, validateMinFeePerGas),
paramtypes.NewParamSetPair(KeyWhitelistedCwCodeHashesForDelegateCall, &p.WhitelistedCwCodeHashesForDelegateCall, validateWhitelistedCwHashesForDelegateCall),
paramtypes.NewParamSetPair(KeyDeliverTxHookWasmGasLimit, &p.DeliverTxHookWasmGasLimit, validateDeliverTxHookWasmGasLimit),
paramtypes.NewParamSetPair(KeyTargetGasUsedPerBlock, &p.TargetGasUsedPerBlock, validateTargetGasUsedPerBlock),
paramtypes.NewParamSetPair(KeyTargetGasUsedPerBlock, &p.TargetGasUsedPerBlock, func(i interface{}) error { return nil }),
}
}

Expand All @@ -90,23 +90,9 @@ func (p Params) Validate() error {
if err := validateBaseFeeAdjustment(p.MaxDynamicBaseFeeDownwardAdjustment); err != nil {
return fmt.Errorf("invalid max dynamic base fee downward adjustment: %s, err: %s", p.MaxDynamicBaseFeeDownwardAdjustment, err)
}
if err := validateTargetGasUsedPerBlock(p.TargetGasUsedPerBlock); err != nil {
return err
}
return validateWhitelistedCwHashesForDelegateCall(p.WhitelistedCwCodeHashesForDelegateCall)
}

func validateTargetGasUsedPerBlock(i interface{}) error {
v, ok := i.(uint64)
if !ok {
return fmt.Errorf("invalid parameter type: %T", i)
}
if v == 0 {
return fmt.Errorf("invalid target gas used per block: must be greater than 0, got %d", v)
}
return nil
}

func validateBaseFeeAdjustment(i interface{}) error {
adjustment, ok := i.(sdk.Dec)
if !ok {
Expand Down

0 comments on commit 81c0d5f

Please sign in to comment.