Panic on invalid unsigned phragmen solution

Lines 1909 to 1920 in 3fe26e6

    
           ensure_none(origin)?; 
        
           Self::check_and_replace_solution( 
        
           	winners, 
        
           	compact_assignments, 
        
           	ElectionCompute::Unsigned, 
        
           	score, 
        
           	era, 
        
           )? 
        
           // TODO: instead of returning an error, panic. This makes the entire produced block 
        
           // invalid. 
        
           // This ensures that block authors will not ever try and submit a solution which is not 
        
           // an improvement, since they will lose their authoring points/rewards.

I'm not sure this is very important in this PR #6106 I see 0.33 weak submission per era.
Moreover weak submission will no longer take weight because it will be refund with #6032

Thus I think it doesn't waste too much.

Also I don't see why there is a security flag here.

related: #4517 (comment)

Yeah now with proper weight refund perhaps this is less of a security issue. But still, no change allows each block author to submit any potentially a garbage solution with no consequence. Even if we refund, this is taking everyone's time. Moreover, they can submit a garbage solution that will not be refunded, and will be rejected due to an issue at the very end (i.e. bogus score). This allows validators to mess with the chain with no consequence, and again waste everyone's time.

Still, you can argue that if they do this, then they will have less weight to pack other transactions in the block, hence they will gain less profit. But I don't think this is a strong enough guarantee. Panic would disincentivize this because they lose their entire authoring block.

I won't run into this issue, but I think it should be closed long term once we have a stabilised offchain phragmen pipeline and it has been battle tested.

Panic would disincentivize this because they lose their entire authoring block.

Panic where? In the extrinsic?

yeah.

We don't have a mechanism that invalidates a full block on panic of an extrinsic ^^

We don't have a mechanism that invalidates a full block on panic of an extrinsic ^^

I think what Gav meant was during import: a transaction panics in block import, then this invalidates the whole block no?

Ahh yes that is correct.

Once #6173 is merged, if we no longer have invalid unsigned solutions, this will be patched thereafter.

I've been doing more analysis after #6173 and we seem to be doing well. I will soon fix this.

Navigation Menu

Repository navigation

Panic on invalid unsigned phragmen solution #5980

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	ensure_none(origin)?;
	Self::check_and_replace_solution(
	winners,
	compact_assignments,
	ElectionCompute::Unsigned,
	score,
	era,
	)?
	// TODO: instead of returning an error, panic. This makes the entire produced block
	// invalid.
	// This ensures that block authors will not ever try and submit a solution which is not
	// an improvement, since they will lose their authoring points/rewards.

Panic on invalid unsigned phragmen solution #5980

Description

Activity

gui1117 commented on May 22, 2020

kianenigma commented on May 22, 2020

bkchr commented on May 23, 2020

kianenigma commented on May 24, 2020

bkchr commented on May 24, 2020

kianenigma commented on May 25, 2020

bkchr commented on May 25, 2020

kianenigma commented on Jun 2, 2020

kianenigma commented on Jun 23, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions