remove ValidateRewardProtocol flag #3946

dustinxie · 2023-10-24T17:27:31Z

Description

in (#3388), validation is added to check the reward action signer, and the change (starting v1.8) is gated by a flag as usual.

It is verified that the validation/check also works for the chain before the feature activation, so the flag can be removed

Fixes #(issue)

Type of change

Please delete options that are not relevant.

[] Bug fix (non-breaking change which fixes an issue)
[] New feature (non-breaking change which adds functionality)
Code refactor or improvement
[] Breaking change (fix or feature that would cause a new or changed behavior of existing functionality)
[] This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

make test
fullsync
[] Other test (please specify)

Test Configuration:

Firmware version:
Hardware:
Toolchain:
SDK:

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

dustinxie · 2023-10-24T17:29:20Z

state/factory/workingset_test.go

+		actions := []action.SealedEnvelope{makeTransferAction(t, 1), makeRewardAction(t, 27)}
+		for _, f := range factories {
+			block := makeBlock(t, hash.ZeroHash256, receiptRoot, digestHash, actions...)
+			require.ErrorContains(f.Validate(zctx, block), "Only producer could create reward")


add a positive test to trigger the error

codecov · 2023-10-24T17:46:35Z

Codecov Report

Merging #3946 (7c401db) into master (e1f0636) will increase coverage by 0.80%.
Report is 96 commits behind head on master.
The diff coverage is 77.33%.

❗ Current head 7c401db differs from pull request most recent head 9603244. Consider uploading reports for the commit 9603244 to get more accurate results

@@            Coverage Diff             @@
##           master    #3946      +/-   ##
==========================================
+ Coverage   75.38%   76.18%   +0.80%     
==========================================
  Files         303      329      +26     
  Lines       25923    28005    +2082     
==========================================
+ Hits        19541    21335    +1794     
- Misses       5360     5572     +212     
- Partials     1022     1098      +76

Files	Coverage Δ
action/candidate_register.go	`90.00% <100.00%> (ø)`
action/candidate_update.go	`89.01% <100.00%> (+0.12%)`	⬆️
action/claimreward.go	`90.62% <100.00%> (-0.42%)`	⬇️
action/depositreward.go	`87.50% <100.00%> (-0.56%)`	⬇️
action/protocol/context.go	`67.66% <100.00%> (+0.49%)`	⬆️
action/protocol/execution/evm/evmstatedbadapter.go	`66.77% <ø> (ø)`
action/protocol/execution/protocol.go	`42.10% <100.00%> (ø)`
action/protocol/rewarding/protocol.go	`80.08% <ø> (+0.67%)`	⬆️
action/protocol/rewarding/reward.go	`90.07% <100.00%> (+0.60%)`	⬆️
...tion/protocol/staking/candidate_buckets_indexer.go	`89.31% <100.00%> (-0.47%)`	⬇️
... and 69 more