Feat(stateless-validation): Rewards for stateless validators

[birchmd]

The goal of this PR is to give rewards to stateless validators (validators which do not produce blocks or chunks but do validate chunks by producing endorsements).

This means updating EpochInfoAggregator to track the expected number of endorsements for each validator as well as how many were actually produced. For the MVP we are not actually tracking endorsements directly, instead we assume that if a chunk was included in a block then all stateless validators for that shard should be rewarded (because there must have been a sufficient number of endorsements for the block producer to include the chunk). However, this logic will need to be iterated on in future versions of the stateless validation protocol. Additionally, the reward calculator required updating to use the new endorsement stats as part of the uptime calculation which forms the basis of the rewards.

Both of those changes are relatively minor. The technical snag in all of this is that the EpochInfoAggregator and other structures related to validator stats are persisted in the node state via borsh serialization. To avoid a large database migration, I have made this update backwards compatible with the old format without the endorsement stats. The logic for this is in core/primitives/src/types/chunk_validator_stats.rs. The legacy variant is distinguished from the new one in the serialization by serializing the one's compliment of the u64 values for the new variant. During deserialization we check if the first u64 is larger than 2^32 - 1 or not. If it is larger then we assume the value is actually a one's compliment and proceed with the new variant deserialization, and otherwise we do the legacy deserialization.

This means that the serialized values can be interpreted improperly if 2^32 (4.29 billion) or more chunks are produced by a single validator during one epoch. I do not expect this to be an issue, but it is a limitation worth documenting.

@Longarithm told me that the DB migration is actually fine here because it's not that much data. So the DB migration of the EpochValidatorInfo column is also implemented.

[codecov]

Codecov Report

Attention: Patch coverage is 75.35934% with 120 lines in your changes are missing coverage. Please review.

Project coverage is 71.10%. Comparing base (16e2321) to head (153d010).
Report is 1 commits behind head on master.

Files	Patch %	Lines
core/store/src/migrations.rs	0.00%	98 Missing ⚠️
chain/epoch-manager/src/lib.rs	79.01%	17 Missing ⚠️
chain/chain/src/runtime/tests.rs	96.07%	0 Missing and 2 partials ⚠️
core/primitives/src/views.rs	0.00%	2 Missing ⚠️
nearcore/src/migrations.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11121      +/-   ##
==========================================
- Coverage   71.10%   71.10%   -0.01%     
==========================================
  Files         772      773       +1     
  Lines      153724   154176     +452     
  Branches   153724   154176     +452     
==========================================
+ Hits       109310   109629     +319     
- Misses      39963    40096     +133     
  Partials     4451     4451              

Flag	Coverage Δ
backward-compatibility	0.24% <0.00%> (-0.01%)	⬇️
db-migration	0.24% <0.00%> (-0.01%)	⬇️
genesis-check	1.43% <0.00%> (-0.01%)	⬇️
integration-tests	36.88% <28.54%> (-0.03%)	⬇️
linux	69.51% <71.25%> (-0.01%)	⬇️
linux-nightly	70.57% <75.15%> (+<0.01%)	⬆️
macos	54.29% <71.25%> (+0.11%)	⬆️
pytests	1.65% <0.00%> (-0.01%)	⬇️
sanity-checks	1.44% <0.00%> (-0.01%)	⬇️
unittests	66.76% <75.35%> (+0.02%)	⬆️
upgradability	0.29% <0.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Longarithm]

If we migrate DB, I don't understand why we don't use ChunkValidatorStatsV2 as ChunkValidatorStats everywhere, I don't think V1 is needed for ChunkValidatorStats.

We can pretend that endorsement stats existed from the beginning and were equal to zero, the logic for computing rewards should be gated by protocol version, so that zero endorsement stats won't cause issues with replayability.

It should simplify code in core/primitives/src/types/chunk_validator_stats.rs. ChunkValidatorStats::V1 { ValidatorStats ... } can be renamed to ChunkValidatorStats::old(ValidatorStats ...).

[birchmd]

why we don't use ChunkValidatorStatsV2 as ChunkValidatorStats everywhere

I think it is important to wrap structs that use Borsh serialization in an enum because it makes it easy to change those data structures in the future without another DB migration. And if we make it an enum any way then I think we might as well include the original variant (it is 16 bytes smaller than the new variant).

the logic for computing rewards should be gated by protocol version

I don't think this is needed because the new logic is identical to the old logic in the case that expected_endorsements == 0. This is why the tests only needed syntactic changes, not logic changes.

ChunkValidatorStats::old(ValidatorStats ...)

I have added a convenience function for this case in 66a8124

[Longarithm]

it is important to wrap structs that use Borsh serialization in an enum

That makes sense, but for structures which are part of network messaging or protocol, like, if changing structure changes a hash. But for ValidatorStats it is not the case.

I don't see what is the usecase of rewriting V1 with V2 using endorsement_stats_mut, or comparing them with v1_v2_eq. And if we ever introduce V3, then we'll have to add comparison and conversion between all pairs of versions. And migration shouldn't be more complex than that; furthermore, migration code can be removed, and even more important, versioned code requires maintenance and understanding. So I'm not sure whether migrations should be avoided at all costs.

From other perspective: if we replace all occurrences of ChunkValidatorStats::V1(stats) with ChunkValidatorStats::V2(production: stats, endorsements: ValidatorStats::default()), it just strictly reduces number of lines of code. And saving 16 bytes for column which takes like 8 MBs whereas other columns take GBs doesn't look important.

[Longarithm]

Some more comments.

UPD: Ideally we should have some integration test checking that if chunk validators skip the work, they are kicked out. As this case may be hard to separate in tests, can be done outside of the PR.

[Longarithm]

As we are increasing number of factors, can we add some comments why overflows don't happen at any point? We recently hit a crash because of that in statelessnet so we need to be careful about that.

[birchmd]

There is test_reward_no_overflow which checks the calculation still works with the anticipated maximum values.

[Longarithm]

Because we touch this code, can we replace &[u64] with &[u64; 2]? It would make it much easier to read.

[birchmd]

Done in fcc4e9a

[Longarithm]

For readability, can we rename these to expected_blocks and expected_chunks? Because in formulas it makes me always wonder whether it is "expected" or "produced".

[birchmd]

Done in da37c4a

[Longarithm]

I think validators responsible for all 3 roles are also worth unit testing, because that's what we will see on MVP - top validators will produce blocks and chunks and validate all the shards.

[birchmd]

test1 has all three roles in test_reward_stateless_validation

[birchmd]

use ChunkValidatorStatsV2 as ChunkValidatorStats everywhere

Done in b1053a8

[Longarithm]

Thank you very much!