From 5b3336b7beb4a16b9f032838e1bd5b1185f6bcd2 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 14 Dec 2020 20:51:43 -0800 Subject: [PATCH] disable gas burning for window post messages While over-estimation fees and miner tips are still paid, gas is no longer burnt for direct, successful window PoSt messages. Usually, gas is burnt to prevent an attacker from spamming the network and to allow clients to "price" messages (using the base fee cap) based on how urgently they need them to be processed. However: 1. Window PoSt is already a "proof of work". 2. Miners need to submit WindowedPoSts on-time so all window post messages are urgent. 3. Work is already under way to move window post verification off-chain (making it effectively free). This change simply introduces the "free" part a bit earlier. --- build/params_2k.go | 1 + build/params_mainnet.go | 2 ++ build/params_testground.go | 1 + chain/vm/burn.go | 9 +++++++-- chain/vm/burn_test.go | 2 +- chain/vm/vm.go | 37 ++++++++++++++++++++++++++++++++++++- 6 files changed, 48 insertions(+), 4 deletions(-) diff --git a/build/params_2k.go b/build/params_2k.go index c86de7ffa23..1155488b482 100644 --- a/build/params_2k.go +++ b/build/params_2k.go @@ -24,6 +24,7 @@ var UpgradeLiftoffHeight = abi.ChainEpoch(-5) const UpgradeKumquatHeight = 15 const UpgradeCalicoHeight = 20 const UpgradePersianHeight = 25 +const UpgradeClausHeight = 30 var DrandSchedule = map[abi.ChainEpoch]DrandEnum{ 0: DrandMainnet, diff --git a/build/params_mainnet.go b/build/params_mainnet.go index 3d8f5e374e6..d34d384f5bb 100644 --- a/build/params_mainnet.go +++ b/build/params_mainnet.go @@ -41,6 +41,8 @@ const UpgradeKumquatHeight = 170000 const UpgradeCalicoHeight = 265200 const UpgradePersianHeight = UpgradeCalicoHeight + (builtin2.EpochsInHour * 60) +const UpgradeClausHeight = 343200 + func init() { policy.SetConsensusMinerMinPower(abi.NewStoragePower(10 << 40)) diff --git a/build/params_testground.go b/build/params_testground.go index 0ee986a7c2f..dbfdf223c8c 100644 --- a/build/params_testground.go +++ b/build/params_testground.go @@ -90,6 +90,7 @@ var ( UpgradeKumquatHeight abi.ChainEpoch = -6 UpgradeCalicoHeight abi.ChainEpoch = -7 UpgradePersianHeight abi.ChainEpoch = -8 + UpgradeClausHeight abi.ChainEpoch = -9 DrandSchedule = map[abi.ChainEpoch]DrandEnum{ 0: DrandMainnet, diff --git a/chain/vm/burn.go b/chain/vm/burn.go index 9f9b95755b7..dd692493622 100644 --- a/chain/vm/burn.go +++ b/chain/vm/burn.go @@ -67,7 +67,7 @@ func ComputeGasOverestimationBurn(gasUsed, gasLimit int64) (int64, int64) { return gasLimit - gasUsed - gasToBurn.Int64(), gasToBurn.Int64() } -func ComputeGasOutputs(gasUsed, gasLimit int64, baseFee, feeCap, gasPremium abi.TokenAmount) GasOutputs { +func ComputeGasOutputs(gasUsed, gasLimit int64, baseFee, feeCap, gasPremium abi.TokenAmount, burn bool) GasOutputs { gasUsedBig := big.NewInt(gasUsed) out := ZeroGasOutputs() @@ -76,7 +76,12 @@ func ComputeGasOutputs(gasUsed, gasLimit int64, baseFee, feeCap, gasPremium abi. baseFeeToPay = feeCap out.MinerPenalty = big.Mul(big.Sub(baseFee, feeCap), gasUsedBig) } - out.BaseFeeBurn = big.Mul(baseFeeToPay, gasUsedBig) + + // If burning is disabled, just skip computing the BaseFeeBurn. However, + // we charge all the other fees regardless. + if burn { + out.BaseFeeBurn = big.Mul(baseFeeToPay, gasUsedBig) + } minerTip := gasPremium if big.Cmp(big.Add(baseFeeToPay, minerTip), feeCap) > 0 { diff --git a/chain/vm/burn_test.go b/chain/vm/burn_test.go index 58e1336057b..e4fc69affd6 100644 --- a/chain/vm/burn_test.go +++ b/chain/vm/burn_test.go @@ -63,7 +63,7 @@ func TestGasOutputs(t *testing.T) { for _, test := range tests { test := test t.Run(fmt.Sprintf("%v", test), func(t *testing.T) { - output := ComputeGasOutputs(test.used, test.limit, baseFee, types.NewInt(test.feeCap), types.NewInt(test.premium)) + output := ComputeGasOutputs(test.used, test.limit, baseFee, types.NewInt(test.feeCap), types.NewInt(test.premium), true) i2s := func(i uint64) string { return fmt.Sprintf("%d", i) } diff --git a/chain/vm/vm.go b/chain/vm/vm.go index 0919b8e8ac4..2baf37a37ac 100644 --- a/chain/vm/vm.go +++ b/chain/vm/vm.go @@ -32,6 +32,7 @@ import ( "github.com/filecoin-project/lotus/chain/actors/adt" "github.com/filecoin-project/lotus/chain/actors/aerrors" "github.com/filecoin-project/lotus/chain/actors/builtin/account" + "github.com/filecoin-project/lotus/chain/actors/builtin/miner" "github.com/filecoin-project/lotus/chain/actors/builtin/reward" "github.com/filecoin-project/lotus/chain/state" "github.com/filecoin-project/lotus/chain/types" @@ -561,7 +562,13 @@ func (vm *VM) ApplyMessage(ctx context.Context, cmsg types.ChainMsg) (*ApplyRet, if gasUsed < 0 { gasUsed = 0 } - gasOutputs := ComputeGasOutputs(gasUsed, msg.GasLimit, vm.baseFee, msg.GasFeeCap, msg.GasPremium) + + burn, err := vm.shouldBurn(st, msg, errcode) + if err != nil { + return nil, xerrors.Errorf("deciding whether should burn failed: %w", err) + } + + gasOutputs := ComputeGasOutputs(gasUsed, msg.GasLimit, vm.baseFee, msg.GasFeeCap, msg.GasPremium, burn) if err := vm.transferFromGasHolder(builtin.BurntFundsActorAddr, gasHolder, gasOutputs.BaseFeeBurn); err != nil { @@ -599,6 +606,34 @@ func (vm *VM) ApplyMessage(ctx context.Context, cmsg types.ChainMsg) (*ApplyRet, }, nil } +func (vm *VM) shouldBurn(st *state.StateTree, msg *types.Message, errcode exitcode.ExitCode) (bool, error) { + // Check to see if we should burn funds. We avoid burning on successful + // window post. This won't catch _indirect_ window post calls, but this + // is the best we can get for now. + // + // This implements FIP-0009: /~https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0009.md + // + // NOTE: This is a stopgap measure and is intended to be reverted once a better solution such as + // /~https://github.com/filecoin-project/FIPs/issues/42 has been is ready. + if vm.blockHeight > build.UpgradeClausHeight && errcode == exitcode.Ok && msg.Method == miner.Methods.SubmitWindowedPoSt { + // Ok, we've checked the _method_, but we still need to check + // the target actor. It would be nice if we could just look at + // the trace, but I'm not sure if that's safe? + if toActor, err := st.GetActor(msg.To); err != nil { + // If the actor wasn't found, we probably deleted it or something. Move on. + if !xerrors.Is(err, types.ErrActorNotFound) { + // Otherwise, this should never fail and something is very wrong. + return false, xerrors.Errorf("failed to lookup target actor: %w", err) + } + } else if builtin.IsStorageMinerActor(toActor.Code) { + // Ok, this is a storage miner and we've processed a window post. Remove the burn. + return false, nil + } + } + + return true, nil +} + func (vm *VM) ActorBalance(addr address.Address) (types.BigInt, aerrors.ActorError) { act, err := vm.cstate.GetActor(addr) if err != nil {