refactor: remove chunk header signature verification from epoch manager

[stedfn]

The MR moves the chunk header signature verification outside of the epoch manager

[codecov]

Codecov Report

Attention: Patch coverage is 88.52459% with 7 lines in your changes missing coverage. Please review.

Project coverage is 70.12%. Comparing base (e2d22c5) to head (e013756).
Report is 15 commits behind head on master.

Files with missing lines	Patch %	Lines
chain/chain/src/signature_verification.rs	91.66%	1 Missing and 2 partials ⚠️
chain/chain/src/chain.rs	50.00%	0 Missing and 2 partials ⚠️
chain/chain/src/validate.rs	83.33%	0 Missing and 1 partial ⚠️
chain/chunks/src/shards_manager_actor.rs	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #12462      +/-   ##
==========================================
+ Coverage   70.08%   70.12%   +0.03%     
==========================================
  Files         840      841       +1     
  Lines      170194   169720     -474     
  Branches   170194   169720     -474     
==========================================
- Hits       119277   119012     -265     
+ Misses      45753    45574     -179     
+ Partials     5164     5134      -30     

Flag	Coverage Δ
backward-compatibility	0.16% <0.00%> (+<0.01%)	⬆️
db-migration	0.16% <0.00%> (+<0.01%)	⬆️
genesis-check	1.29% <0.00%> (+<0.01%)	⬆️
linux	69.35% <88.52%> (+0.04%)	⬆️
linux-nightly	69.71% <88.52%> (+0.04%)	⬆️
pytests	1.59% <0.00%> (+<0.01%)	⬆️
sanity-checks	1.40% <0.00%> (+<0.01%)	⬆️
unittests	69.94% <88.52%> (+0.03%)	⬆️
upgradability	0.20% <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.

[shreyan-gupta]

I'm assuming we would be moving other verify functions to this file soon?

[stedfn]

I'm assuming we would be moving other verify functions to this file soon?

Yes, that's what I plan

[pugachAG]

could you please add epoch_id as a parameter to verify_chunk_header_signature_with_epoch_manager as it was in the original epoch manager API, so we don't have to check should_verify_signature on the client in shards manager

[stedfn]

@pugachAG I am not sure I understand. Do you mean adding the epoch manager as a parameter to verify_chunk_header_signature and checking there whether signatures should be verified?

[pugachAG]

@stedfn previously we had

fn verify_chunk_header_signature(
        &self,
        header: &ShardChunkHeader,
        epoch_id: &EpochId,
        last_known_hash: &CryptoHash,
    ) -> Result<bool, Error> {

and now we no longer pass epoch_id and instead get it from the previous block:

pub fn verify_chunk_header_signature_with_epoch_manager(
    epoch_manager: &dyn EpochManagerAdapter,
    chunk_header: &ShardChunkHeader,
    parent_hash: &CryptoHash,
) -> Result<bool, Error> {

What I suggest is to preserve passing epoch_id as a parameter. This way you can avoid let valid_signature = !self.epoch_manager.should_validate_signatures() || verify_chunk_header_signature( and use verify_chunk_header_signature_with_epoch_manager instead

[pugachAG]

LGTM, please address the comment before merging