From 80fccba93fe50af84f4cdd256dfddca9382fff95 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Fri, 10 Feb 2023 04:48:20 -0800 Subject: [PATCH] fix: gas: update ffi & correct the message inclusion cost in nv18 (#10228) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Raúl Kripalani --- chain/vm/gas.go | 10 +++ extern/filecoin-ffi | 2 +- itests/fevm_test.go | 4 +- itests/gas_estimation_test.go | 117 +++++++++++++++++++++++++++++++++- 4 files changed, 129 insertions(+), 4 deletions(-) diff --git a/chain/vm/gas.go b/chain/vm/gas.go index c9007d3f1..cb0c5def9 100644 --- a/chain/vm/gas.go +++ b/chain/vm/gas.go @@ -213,6 +213,16 @@ var Prices = map[abi.ChainEpoch]Pricelist{ verifyReplicaUpdate: 36316136, }, + build.UpgradeHyggeHeight: &pricelistV0{ + computeGasMulti: 1, + storageGasMulti: 1300, // only applies to messages/return values. + + onChainMessageComputeBase: 38863 + 475000, // includes the actor update cost + onChainMessageStorageBase: 36, + onChainMessageStoragePerByte: 1, + + onChainReturnValuePerByte: 1, + }, } // PricelistByEpoch finds the latest prices for the given epoch diff --git a/extern/filecoin-ffi b/extern/filecoin-ffi index 32df7d1f1..9e4fe7547 160000 --- a/extern/filecoin-ffi +++ b/extern/filecoin-ffi @@ -1 +1 @@ -Subproject commit 32df7d1f15e46406c5efafb5de5c2c70ec34d685 +Subproject commit 9e4fe75471f635a4ba91f17ee76f75200eb91de8 diff --git a/itests/fevm_test.go b/itests/fevm_test.go index 51aff1aac..6cdd440dd 100644 --- a/itests/fevm_test.go +++ b/itests/fevm_test.go @@ -58,7 +58,7 @@ func buildInputFromuint64(number uint64) []byte { // recursive delegate calls that fail due to gas limits are currently getting to 229 iterations // before running out of gas func recursiveDelegatecallFail(ctx context.Context, t *testing.T, client *kit.TestFullNode, filename string, count uint64) { - expectedIterationsBeforeFailing := int(229) + expectedIterationsBeforeFailing := int(220) fromAddr, idAddr := client.EVM().DeployContractFromFilename(ctx, filename) t.Log("recursion count - ", count) inputData := buildInputFromuint64(count) @@ -158,7 +158,7 @@ func TestFEVMRecursiveDelegatecallCount(t *testing.T) { ctx, cancel, client := kit.SetupFEVMTest(t) defer cancel() - highestSuccessCount := uint64(235) + highestSuccessCount := uint64(225) filename := "contracts/RecursiveDelegeatecall.hex" recursiveDelegatecallSuccess(ctx, t, client, filename, uint64(1)) diff --git a/itests/gas_estimation_test.go b/itests/gas_estimation_test.go index 42cad8611..24013c885 100644 --- a/itests/gas_estimation_test.go +++ b/itests/gas_estimation_test.go @@ -2,16 +2,23 @@ package itests import ( "context" + "math" "testing" "time" "github.com/stretchr/testify/require" + "github.com/filecoin-project/go-address" + "github.com/filecoin-project/go-state-types/abi" "github.com/filecoin-project/go-state-types/big" + "github.com/filecoin-project/go-state-types/exitcode" "github.com/filecoin-project/lotus/api" + "github.com/filecoin-project/lotus/build" + "github.com/filecoin-project/lotus/chain/actors/builtin" "github.com/filecoin-project/lotus/chain/actors/builtin/account" "github.com/filecoin-project/lotus/chain/types" + "github.com/filecoin-project/lotus/chain/vm" "github.com/filecoin-project/lotus/itests/kit" ) @@ -37,8 +44,9 @@ func TestEstimateGasNoFunds(t *testing.T) { sm, err := client.MpoolPushMessage(ctx, msg, nil) require.NoError(t, err) - _, err = client.StateWaitMsg(ctx, sm.Cid(), 3, api.LookbackNoLimit, true) + ret, err := client.StateWaitMsg(ctx, sm.Cid(), 3, api.LookbackNoLimit, true) require.NoError(t, err) + require.True(t, ret.Receipt.ExitCode.IsSuccess()) // Make sure we can estimate gas even if we have no funds. msg2 := &types.Message{ @@ -52,3 +60,110 @@ func TestEstimateGasNoFunds(t *testing.T) { require.NoError(t, err) require.NotZero(t, limit) } + +// Make sure that we correctly calculate the inclusion cost. Especially, make sure the FVM and Lotus +// agree and that: +// 1. The FVM will never charge _less_ than the inclusion cost. +// 2. The FVM will never fine a storage provider for including a message that costs exactly the +// inclusion cost. +func TestEstimateInclusion(t *testing.T) { + ctx := context.Background() + + kit.QuietMiningLogs() + + // We need this to be "correct" in this test so that lotus can get the correct gas value + // (which, unfortunately, looks at the height and not the current network version). + oldPrices := vm.Prices + vm.Prices = map[abi.ChainEpoch]vm.Pricelist{ + 0: oldPrices[build.UpgradeHyggeHeight], + } + t.Cleanup(func() { vm.Prices = oldPrices }) + + client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs()) + ens.InterconnectAll().BeginMining(10 * time.Millisecond) + + // First, try sending a message that should have no fees beyond the inclusion cost. I.e., it + // does absolutely nothing: + msg := &types.Message{ + From: client.DefaultKey.Address, + To: client.DefaultKey.Address, + Value: big.Zero(), + GasLimit: 0, + GasFeeCap: abi.NewTokenAmount(10000), + GasPremium: big.Zero(), + } + + burntBefore, err := client.WalletBalance(ctx, builtin.BurntFundsActorAddr) + require.NoError(t, err) + balanceBefore, err := client.WalletBalance(ctx, client.DefaultKey.Address) + require.NoError(t, err) + + // Sign the message and compute the correct inclusion cost. + var smsg *types.SignedMessage + for i := 0; ; i++ { + var err error + smsg, err = client.WalletSignMessage(ctx, client.DefaultKey.Address, msg) + require.NoError(t, err) + estimatedGas := vm.PricelistByEpoch(math.MaxInt).OnChainMessage(smsg.ChainLength()).Total() + if estimatedGas == msg.GasLimit { + break + } + // Try 10 times to get the right gas value. + require.Less(t, i, 10, "unable to estimate gas: %s != %s", estimatedGas, msg.GasLimit) + msg.GasLimit = estimatedGas + } + + cid, err := client.MpoolPush(ctx, smsg) + require.NoError(t, err) + ret, err := client.StateWaitMsg(ctx, cid, 3, api.LookbackNoLimit, true) + require.NoError(t, err) + require.True(t, ret.Receipt.ExitCode.IsSuccess()) + require.Equal(t, msg.GasLimit, ret.Receipt.GasUsed) + + // Then try sending a message of the same size that tries to create an actor. This should + // get successfully included, but fail with out of gas: + + // Mutate the last byte to get a new address of the same length. + toBytes := msg.To.Bytes() + toBytes[len(toBytes)-1] += 1 //nolint:golint + newAddr, err := address.NewFromBytes(toBytes) + require.NoError(t, err) + + msg.Nonce = 1 + msg.To = newAddr + smsg, err = client.WalletSignMessage(ctx, client.DefaultKey.Address, msg) + require.NoError(t, err) + + cid, err = client.MpoolPush(ctx, smsg) + require.NoError(t, err) + ret, err = client.StateWaitMsg(ctx, cid, 3, api.LookbackNoLimit, true) + require.NoError(t, err) + require.Equal(t, ret.Receipt.ExitCode, exitcode.SysErrOutOfGas) + require.Equal(t, msg.GasLimit, ret.Receipt.GasUsed) + + // Now make sure that the client is the only contributor to the burnt funds actor (the + // miners should not have been fined for either message). + + burntAfter, err := client.WalletBalance(ctx, builtin.BurntFundsActorAddr) + require.NoError(t, err) + balanceAfter, err := client.WalletBalance(ctx, client.DefaultKey.Address) + require.NoError(t, err) + + burnt := big.Sub(burntAfter, burntBefore) + spent := big.Sub(balanceBefore, balanceAfter) + + require.Equal(t, burnt, spent) + + // Finally, try to submit a message with too little gas. This should fail. + + msg.Nonce = 2 + msg.To = msg.From + msg.GasLimit -= 1 //nolint:golint + + smsg, err = client.WalletSignMessage(ctx, client.DefaultKey.Address, msg) + require.NoError(t, err) + + _, err = client.MpoolPush(ctx, smsg) + require.ErrorContains(t, err, "will not be included in a block") + require.ErrorContains(t, err, "cannot be less than the cost of storing a message") +}