From d192b821a97f4781a279cf72eb33cb6d2aec93c2 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Thu, 7 Jul 2022 13:06:51 -0700 Subject: [PATCH] fix: gas: estimate gas with a zero base-fee Otherwise, an account will need funds to estimate the max possible gas a message could take (which is usually the block gas limit). This does mean gas estimation no longer checks if the sending account has enough funds to cover the message cost, but MpoolPush will now do this. --- .circleci/config.yml | 5 ++ chain/stmgr/call.go | 19 +++++- itests/deals_retry_deal_no_funds_test.go | 76 +++--------------------- itests/gas_estimation_test.go | 54 +++++++++++++++++ node/impl/full/gas.go | 4 +- node/impl/full/mpool.go | 6 +- 6 files changed, 92 insertions(+), 72 deletions(-) create mode 100644 itests/gas_estimation_test.go diff --git a/.circleci/config.yml b/.circleci/config.yml index 914fa2ffd..75846029a 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -858,6 +858,11 @@ workflows: suite: itest-deals target: "./itests/deals_test.go" + - test: + name: test-itest-gas_estimation + suite: itest-gas_estimation + target: "./itests/gas_estimation_test.go" + - test: name: test-itest-gateway suite: itest-gateway diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go index 29b44fda1..d066ddb16 100644 --- a/chain/stmgr/call.go +++ b/chain/stmgr/call.go @@ -12,6 +12,7 @@ import ( "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/crypto" "github.com/filecoin-project/lotus/api" @@ -156,6 +157,10 @@ func (sm *StateManager) CallWithGas(ctx context.Context, msg *types.Message, pri ctx, span := trace.StartSpan(ctx, "statemanager.CallWithGas") defer span.End() + // Copy the message as we'll be modifying the nonce. + msgCopy := *msg + msg = &msgCopy + if ts == nil { ts = sm.cs.GetHeaviestTipSet() @@ -273,9 +278,21 @@ func (sm *StateManager) CallWithGas(ctx context.Context, msg *types.Message, pri } + // If the fee cap is set to zero, make gas free. + if msg.GasFeeCap.NilOrZero() { + // Now estimate with a new VM with no base fee. + vmopt.BaseFee = big.Zero() + vmopt.StateBase = stateCid + + vmi, err = sm.newVM(ctx, vmopt) + if err != nil { + return nil, xerrors.Errorf("failed to set up estimation vm: %w", err) + } + } + ret, err := vmi.ApplyMessage(ctx, msgApply) if err != nil { - return nil, xerrors.Errorf("apply message failed: %w", err) + return nil, xerrors.Errorf("gas estimation failed: %w", err) } var errs string diff --git a/itests/deals_retry_deal_no_funds_test.go b/itests/deals_retry_deal_no_funds_test.go index 317538cf1..98f99a4a2 100644 --- a/itests/deals_retry_deal_no_funds_test.go +++ b/itests/deals_retry_deal_no_funds_test.go @@ -28,6 +28,15 @@ var ( ) func TestDealsRetryLackOfFunds(t *testing.T) { + t.Run("cover-gas", func(t *testing.T) { + testDealsRetryLackOfFunds(t, types.NewInt(1020000000000)) + }) + t.Run("empty", func(t *testing.T) { + testDealsRetryLackOfFunds(t, types.NewInt(1)) + }) +} + +func testDealsRetryLackOfFunds(t *testing.T, publishStorageAccountFunds abi.TokenAmount) { //stm: @CHAIN_SYNCER_LOAD_GENESIS_001, @CHAIN_SYNCER_FETCH_TIPSET_001, //stm: @CHAIN_SYNCER_START_001, @CHAIN_SYNCER_SYNC_001, @BLOCKCHAIN_BEACON_VALIDATE_BLOCK_VALUES_01 //stm: @CHAIN_SYNCER_COLLECT_CHAIN_001, @CHAIN_SYNCER_COLLECT_HEADERS_001, @CHAIN_SYNCER_VALIDATE_TIPSET_001 @@ -61,7 +70,6 @@ func TestDealsRetryLackOfFunds(t *testing.T) { })), ) - publishStorageAccountFunds := types.NewInt(1020000000000) minerFullNode, clientFullNode, miner, ens := kit.EnsembleTwoOne(t, kit.Account(publishStorageDealKey, publishStorageAccountFunds), kit.ConstructorOpts(opts), kit.MockProofs(), eightMBSectorsOpt) kit.QuietMiningLogs() @@ -178,69 +186,3 @@ func TestDealsRetryLackOfFunds_blockInPublishDeal(t *testing.T) { case <-time.After(time.Second * 15): } } - -func TestDealsRetryLackOfFunds_belowLimit(t *testing.T) { - //stm: @CHAIN_SYNCER_LOAD_GENESIS_001, @CHAIN_SYNCER_FETCH_TIPSET_001, - //stm: @CHAIN_SYNCER_START_001, @CHAIN_SYNCER_SYNC_001, @BLOCKCHAIN_BEACON_VALIDATE_BLOCK_VALUES_01 - //stm: @CHAIN_SYNCER_COLLECT_CHAIN_001, @CHAIN_SYNCER_COLLECT_HEADERS_001, @CHAIN_SYNCER_VALIDATE_TIPSET_001 - //stm: @CHAIN_SYNCER_NEW_PEER_HEAD_001, @CHAIN_SYNCER_VALIDATE_MESSAGE_META_001, @CHAIN_SYNCER_STOP_001 - //stm: @CLIENT_STORAGE_DEALS_LIST_IMPORTS_001 - ctx := context.Background() - kit.QuietMiningLogs() - - // Allow 8MB sectors - eightMBSectorsOpt := kit.SectorSize(8 << 20) - - publishStorageDealKey, err := key.GenerateKey(types.KTSecp256k1) - require.NoError(t, err) - - opts := node.Options( - node.Override(new(*storageadapter.DealPublisher), - storageadapter.NewDealPublisher(nil, storageadapter.PublishMsgConfig{ - Period: publishPeriod, - MaxDealsPerMsg: maxDealsPerMsg, - }), - ), - node.Override(new(*ctladdr.AddressSelector), modules.AddressSelector(&config.MinerAddressConfig{ - DealPublishControl: []string{ - publishStorageDealKey.Address.String(), - }, - DisableOwnerFallback: true, - DisableWorkerFallback: true, - })), - ) - - publishStorageAccountFunds := types.NewInt(1) - minerFullNode, clientFullNode, miner, ens := kit.EnsembleTwoOne(t, kit.Account(publishStorageDealKey, publishStorageAccountFunds), kit.ConstructorOpts(opts), kit.MockProofs(), eightMBSectorsOpt) - - kit.QuietMiningLogs() - - ens. - Start(). - InterconnectAll(). - BeginMining(blockTime) - - _, err = minerFullNode.WalletImport(ctx, &publishStorageDealKey.KeyInfo) - require.NoError(t, err) - - miner.SetControlAddresses(publishStorageDealKey.Address) - - dh := kit.NewDealHarness(t, clientFullNode, miner, miner) - - res, _ := clientFullNode.CreateImportFile(ctx, 0, 4<<20) // 4MiB file. - list, err := clientFullNode.ClientListImports(ctx) - require.NoError(t, err) - require.Len(t, list, 1) - require.Equal(t, res.Root, *list[0].Root) - - dp := dh.DefaultStartDealParams() - dp.Data.Root = res.Root - dp.FastRetrieval = true - dp.EpochPrice = abi.NewTokenAmount(62500000) // minimum asking price. - deal := dh.StartDeal(ctx, dp) - - err = dh.ExpectDealFailure(ctx, deal, "Actor balance less than needed") - if err != nil { - t.Fatal(err) - } -} diff --git a/itests/gas_estimation_test.go b/itests/gas_estimation_test.go new file mode 100644 index 000000000..52d52be30 --- /dev/null +++ b/itests/gas_estimation_test.go @@ -0,0 +1,54 @@ +package itests + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/filecoin-project/go-state-types/big" + + "github.com/filecoin-project/lotus/api" + "github.com/filecoin-project/lotus/chain/actors/builtin/account" + "github.com/filecoin-project/lotus/chain/types" + "github.com/filecoin-project/lotus/itests/kit" +) + +func TestEstimateGasNoFunds(t *testing.T) { + ctx := context.Background() + + kit.QuietMiningLogs() + + client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs()) + ens.InterconnectAll().BeginMining(10 * time.Millisecond) + + // create a new address + addr, err := client.WalletNew(ctx, types.KTBLS) + require.NoError(t, err) + + // Create that address. + msg := &types.Message{ + From: client.DefaultKey.Address, + To: addr, + Value: big.Zero(), + } + + sm, err := client.MpoolPushMessage(ctx, msg, nil) + require.NoError(t, err) + + _, err = client.StateWaitMsg(ctx, sm.Cid(), 3, api.LookbackNoLimit, true) + require.NoError(t, err) + + // Create that address. + msg2 := &types.Message{ + From: addr, + To: client.DefaultKey.Address, + Method: account.Methods.PubkeyAddress, + Value: big.Zero(), + } + + limit, err := client.GasEstimateGasLimit(ctx, msg2, types.EmptyTSK) + require.NoError(t, err) + require.NotZero(t, limit) +} diff --git a/node/impl/full/gas.go b/node/impl/full/gas.go index 1e8a00922..c179c27d6 100644 --- a/node/impl/full/gas.go +++ b/node/impl/full/gas.go @@ -258,8 +258,8 @@ func gasEstimateGasLimit( ) (int64, error) { msg := *msgIn msg.GasLimit = build.BlockGasLimit - msg.GasFeeCap = types.NewInt(uint64(build.MinimumBaseFee) + 1) - msg.GasPremium = types.NewInt(1) + msg.GasFeeCap = big.Zero() + msg.GasPremium = big.Zero() fromA, err := smgr.ResolveToKeyAddress(ctx, msgIn.From, currTs) if err != nil { diff --git a/node/impl/full/mpool.go b/node/impl/full/mpool.go index 2c9451079..e7a461784 100644 --- a/node/impl/full/mpool.go +++ b/node/impl/full/mpool.go @@ -9,6 +9,7 @@ import ( "golang.org/x/xerrors" "github.com/filecoin-project/go-address" + "github.com/filecoin-project/go-state-types/big" "github.com/filecoin-project/lotus/api" "github.com/filecoin-project/lotus/chain/messagepool" @@ -178,8 +179,9 @@ func (a *MpoolAPI) MpoolPushMessage(ctx context.Context, msg *types.Message, spe return nil, xerrors.Errorf("mpool push: getting origin balance: %w", err) } - if b.LessThan(msg.Value) { - return nil, xerrors.Errorf("mpool push: not enough funds: %s < %s", b, msg.Value) + requiredFunds := big.Add(msg.Value, msg.RequiredFunds()) + if b.LessThan(requiredFunds) { + return nil, xerrors.Errorf("mpool push: not enough funds: %s < %s", b, requiredFunds) } // Sign and push the message