add slashing reward mechanism #789

zixuanzh · 2020-01-08T05:30:06Z

per mechanism in Slashing Mechanism #383
add validation checks to consensus fault proofs

TODO:

define arithmetic precision and int types
pledge collateral slashing from detected fault (equivalent of UncommittedPowerFault)

anorth · 2020-01-08T08:35:44Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor_code.go

@@ -180,7 +180,7 @@ func (a *StoragePowerActorCode_I) OnMinerSurprisePoStFailure(rt Runtime, numCons
 	}
 }

-func (a *StoragePowerActorCode_I) ReportConsensusFault(rt Runtime, slasherAddr addr.Address, faultType ConsensusFaultType, proof []block.Block) {
+func (a *StoragePowerActorCode_I) ReportConsensusFault(rt Runtime, faultType ConsensusFaultType, proof []block.BlockHeader) {


I hope that we can remove visibility of the BlockHeader type here and instead delegate the fault checking to the runtime. I'll file an issue for it.

the check here was supposed to be done in ExpectedConsensus. Does this mean that that logic will now also live in the runtime? I am inclined to leave this as a follow up, there might be other places in actor that require similar validation cc @sternhenri

Mmmh I now understand your question from yesterday (out of band) @zixuanzh. Yeah, we can make it work that way, we'll just need to special case the message validation in the runtime in order to run this check...

So maybe change this to ReportVerifiedConsensusFault passing in the slasheeAddr as an arg (instead of proof) @zixuanzh and we'll write the verif code in the runtime with the proof which then calls this method.

anorth · 2020-01-08T08:39:10Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor_code.go

+		collateralToSlash := st._getPledgeSlashForConsensusFault(currPledge, faultType)
+		slasherShare := _getConsensusFaultSlasherShare(elapsedEpoch)
+		Assert(slasherShare <= 1 && slasherShare > 0)
+		amountToSlasher := actor.TokenAmount(slasherShare) * collateralToSlash


Blocking: TokenAmount is an integer so the cast will clamp to either 0 or 1, which isn't what we want.

I have a strong preference for reworking this arithmetic in terms of bigints. Rather than _getConsensusFaultSlasherShare returning a float, have it accept collateralToSlash as a parameter and compute the share using integer multiplication and division. Perhaps we can represent the constants as numerator/denominator ratios.

I'm not sure on how to do BigInt arithmetic correctly atm, so I have updated the logic and left them in comment form.

anorth · 2020-01-08T08:41:34Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor_code.go

+		UpdateRelease(rt, h, st)
+
+		// reward slasher
+		rt.SendFunds(slasherAddr, amountToSlasher)


Blocking: without subtracting amountToSlasher from the balance table this will burn too much funds when the actor is deleted.

sorry about this, this has been fixed.

sternhenri

a proposed fix, but otherwise LGTM

sternhenri · 2020-01-08T14:36:53Z

src/algorithms/expected_consensus/_index.md

@@ -201,8 +201,7 @@ This is detectable when a given miner submits two blocks that satisfy any of the
 - (3) `parent-grinding fault`: one block's parent is a Tipset that provably should have included a given block but does not. While it cannot be proven that a missing block was willfully omitted in general (i.e. network latency could simply mean the miner did not receive a particular block), it can when a miner has successfully mined a block two epochs in a row and omitted one. That is, this condition should be evoked when a miner omits their own prior block. When a miner's block at epoch e + 1 references a Tipset that does not include the block they mined at e both blocks can be submitted to prove this fault.
 {{< diagram src="diagrams/parent_grinding.dot.svg" title="Parent-Grinding fault" >}}

-Any node that detects any of the above events should submit both block headers to the `StoragePowerActor`'s `ReportConsensusFault` method. The "slasher" will receive a portion (TODO: define how much) of the offending miner's {{<sref pledge_collateral>}} as a reward for notifying the network of the fault.
-(TODO: FIP of submitting commitments to block headers to prevent miners censoring slashers in order to gain rewards).
+Any node that detects any of the above events should submit both block headers to the `StoragePowerActor`'s `ReportConsensusFault` method. The "slasher" will receive a portion of the offending miner's {{<sref pledge_collateral>}} as a reward for notifying the network of the fault. Consensus faults (except for `uncommitted power fault` below which falls under storage faults with impact on consensus) will tentatively result in all pledge collateral being slashed and the miner removed from the power table. Some portion of the pledge collateral is given to the slasher as a function of some initial share (`SLASHER_INITIAL_SHARE`) and growth rate (`SLASHER_SHARE_GROWTH_RATE`). Slasher's share of the slashed collateral increases as block elapses since the block when the fault is committed. Default growth rate results in slasher's share reaches 1 after 30 blocks. However, only the first slasher gets its share of the pledge collateral and the remaining pledge collateral will be burned. The longer a slasher waits, the higher the likelihood that the slashed collateral will be claimed by another slasher.


I like this incentive structure. Note that 30 blocks will be 22.5 minutes which seems a bit short to me. I could see slashes coming on the order of finality/2, so closer to something like 250 blocks or something...

will update this to 250 blocks for now but lets keep in mind that they are placeholders and we should examine them more carefully during testnet

reaplace number with the variable name and add variable to network_params.go?

250 is derived from variables (growth rate and initial share) in network params

sternhenri · 2020-01-08T14:46:34Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor_code.go

@@ -180,7 +180,7 @@ func (a *StoragePowerActorCode_I) OnMinerSurprisePoStFailure(rt Runtime, numCons
 	}
 }

-func (a *StoragePowerActorCode_I) ReportConsensusFault(rt Runtime, slasherAddr addr.Address, faultType ConsensusFaultType, proof []block.Block) {
+func (a *StoragePowerActorCode_I) ReportConsensusFault(rt Runtime, faultType ConsensusFaultType, proof []block.BlockHeader) {


Mmmh I now understand your question from yesterday (out of band) @zixuanzh. Yeah, we can make it work that way, we'll just need to special case the message validation in the runtime in order to run this check...

So maybe change this to ReportVerifiedConsensusFault passing in the slasheeAddr as an arg (instead of proof) @zixuanzh and we'll write the verif code in the runtime with the proof which then calls this method.

sternhenri · 2020-01-08T14:56:06Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor_state.go

+	TODO()
+	// BigInt Operation
+	// var growthRate = node_base.SLASHER_SHARE_GROWTH_RATE_NUM / node_base.SLASHER_SHARE_GROWTH_RATE_DENOM
+	// var multiplier = growthRate^elapsedEpoch


you probably want (1+growthRate)^elapsedEpoch - 1 otherwise your number will get smaller with elapsed epochs.

ah growth rate is greater than 1 here so we good.

nit: feels to me a growth rate should be between 0 and 1

sternhenri · 2020-01-08T14:57:53Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor_state.go

+	}
+}
+
+func _getConsensusFaultSlasherReward(elapsedEpoch block.ChainEpoch, totalPledge actor.TokenAmount) actor.TokenAmount {


totalPledge should maybe be slashedCollat since it may not be the total pledge.

sternhenri · 2020-01-08T15:01:07Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor_state.go

+func _getConsensusFaultSlasherReward(elapsedEpoch block.ChainEpoch, totalPledge actor.TokenAmount) actor.TokenAmount {
+	TODO()
+	// BigInt Operation
+	// var growthRate = node_base.SLASHER_SHARE_GROWTH_RATE_NUM / node_base.SLASHER_SHARE_GROWTH_RATE_DENOM


float issue here but no clean way around that I can tell...

zixuanzh requested a review from anorth January 8, 2020 05:35

anorth requested changes Jan 8, 2020

View reviewed changes

anorth approved these changes Jan 8, 2020

View reviewed changes

sternhenri approved these changes Jan 8, 2020

View reviewed changes

zixuanzh added 5 commits January 8, 2020 11:54

add slashing reward mechanism

00f1732

address review comments

3254e85

address hs review comments

0c033b0

update valid consensus fault condition

b44b203

minor fix to indices to match interface

218d29a

zixuanzh force-pushed the slashing-reward branch from 271cc3e to 218d29a Compare January 8, 2020 20:01

zixuanzh merged commit 165c80c into master Jan 8, 2020

zixuanzh deleted the slashing-reward branch January 8, 2020 20:03

zixuanzh mentioned this pull request Jan 9, 2020

Slashing Mechanism #383

Closed

3 tasks

sternhenri mentioned this pull request Jan 9, 2020

Explicitly add parent tipset to slashing function argument #412

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add slashing reward mechanism #789

add slashing reward mechanism #789

zixuanzh commented Jan 8, 2020 •

edited

Loading

anorth Jan 8, 2020

zixuanzh Jan 8, 2020

sternhenri Jan 8, 2020

anorth Jan 8, 2020

zixuanzh Jan 8, 2020

anorth Jan 8, 2020

zixuanzh Jan 8, 2020

sternhenri left a comment

sternhenri Jan 8, 2020

zixuanzh Jan 8, 2020

sternhenri Jan 8, 2020

zixuanzh Jan 8, 2020

sternhenri Jan 8, 2020

sternhenri Jan 8, 2020

zixuanzh Jan 8, 2020

sternhenri Jan 8, 2020

sternhenri Jan 8, 2020

sternhenri Jan 8, 2020

add slashing reward mechanism #789

add slashing reward mechanism #789

Conversation

zixuanzh commented Jan 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sternhenri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zixuanzh commented Jan 8, 2020 •

edited

Loading