Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable gas burning for window post messages #5200

Merged
merged 1 commit into from
Dec 16, 2020
Merged

Conversation

arajasek
Copy link
Contributor

@arajasek arajasek commented Dec 16, 2020

Implements https://github.com/filecoin-project/FIPs/blob/master/FIPS/fip-0009.md

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.

Upgrade epoch set for Monday, Dec 21st, 18:00 PST.

chain/vm/vm.go Outdated Show resolved Hide resolved
chain/vm/vm.go Outdated Show resolved Hide resolved
@anorth
Copy link
Member

anorth commented Dec 16, 2020

Link to the FIP?

@arajasek
Copy link
Contributor Author

@anorth Good call! Edited the description.

Copy link
Member

@anorth anorth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, only minor suggestions about making this more self describing. Burn is a very generic word, used for other things too.

@@ -41,6 +41,8 @@ const UpgradeKumquatHeight = 170000
const UpgradeCalicoHeight = 265200
const UpgradePersianHeight = UpgradeCalicoHeight + (builtin2.EpochsInHour * 60)

const UpgradeClausHeight = 343200
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: it would be helpful to readers to include datestamps alongside these.

chain/vm/burn.go Outdated
@@ -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,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"network transaction fee" would be more descriptive than burn, here and for the parameter name.

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.
@arajasek arajasek merged commit f2aea11 into master Dec 16, 2020
@arajasek arajasek deleted the release/v1.3.0 branch December 16, 2020 05:23
Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
impact/consensus Impact: Consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants