-
Notifications
You must be signed in to change notification settings - Fork 27
migrate and enhance penalties subsuite from chain-validation. #108
Conversation
TipsetFunc: minerPenalized(3, func(v *TipsetVectorBuilder) { | ||
v.StagedMessages.SetDefaults(GasLimit(1_000_000_000), GasPremium(0), GasFeeCap(200)) | ||
|
||
from := MustNewBLSAddr(1000) // random |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where / how do we specify initial balance of this sending actor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a deliberately inexistent actor for the purposes of this test case.
// Accounts returns all accounts that have been registered through | ||
// Account() / AccountN(). | ||
// | ||
// Miner owner and worker accounts, despite being accounts in the strict sense | ||
// of the word, are not returned here. You can get them through Miners(). | ||
// | ||
// Similarly, account actors registered through CreateActor() in a bare form are | ||
// not returned here. | ||
func (a *Actors) Accounts() []Account { | ||
return a.accounts | ||
} | ||
|
||
// Miners returns all miners that have been registered through | ||
// Miner() / MinerN(), along with their owner and worker account addresses. | ||
// | ||
// Miners registered through CreateActor() in a bare form are not returned here. | ||
func (a *Actors) Miners() []Miner { | ||
return a.miners | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now you can get accounts and miners separately, each with their own struct containing the relevant data.
// StagedMessages is a staging area for messages. | ||
StagedMessages *Messages | ||
// StateTracker is used for staging messages, and it's scrapped and replaced | ||
// by a fork at the PreRoot when committing applies. | ||
StateTracker *StateTracker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified the temporary state tracker construction here. We now just have one state tracker, and we replace it in CommitApplies
by recovering the pre-state.
// sender's account as a result of applying this message. | ||
func CalculateDeduction(am *ApplicableMessage) big.Int { | ||
func CalculateSenderDeduction(am *ApplicableMessage) big.Int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More explicit naming to reduce cognitive load.
func GetMinerReward(am *ApplicableMessage) abi.TokenAmount { | ||
gasLimit := big.NewInt(am.Message.GasLimit) | ||
gasPremium := am.Message.GasPremium | ||
return big.Mul(gasLimit, gasPremium) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calculating the miner penalty is not as straightforward, and this function wasn't even used in chain-validation. Nuking it.
// gas limit and the gas actually used. | ||
func CalculateBurn(gasLimit int64, gasUsed int64) big.Int { | ||
func CalculateBurntGas(am *ApplicableMessage) big.Int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More explicit naming. This calculates burnt gas. Miner penalties are also burnt, and I needed to disambiguate.
if gasUsed != 0 { | ||
overestimateGas = big.Div(overestimateGas, big.NewInt(gasUsed)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoiding divisions by zero.
@@ -115,7 +115,7 @@ func (m *Messages) ApplyOne(am *ApplicableMessage) { | |||
} | |||
// verify that preceding messages have been applied. | |||
// this will abort if unsatisfied. | |||
m.bc.Assert.Nil(other.Result, "preceding messages must have been applied when calling Apply*; index of first unapplied: %d", i) | |||
m.bc.Assert.NotNil(other.Result, "preceding messages must have been applied when calling Apply*; index of first unapplied: %d", i) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a genuine bug.
No description provided.