feat(mirror): add a GetShardChunk view client type and call it in mirror subcommand

[marcelo-gonzalez]

Before this change, the only way to get a chunk from the view client actor is to send a GetChunk message which returns a ChunkView. This is what we do when running neard mirror run with the --online-source flag, which is used to start a source chain node that applies mainnet blocks so that we don't run out of transactions. This is normally completely fine, but there is a problem with Deploy Contract actions, where the deploy contract action in the ChunkView type actually only contains the hash of the contract code instead of the whole thing, which means that we can't mirror any deploy contract transactions.

To fix it, we can add a new GetShardChunk view client type that is the same as the existing GetChunk except that it returns the full shard chunk. It's a much smaller change with less refactoring involved to just add a new type, but it could be worth changing the existing GetChunk behavior to just return the full chunk, and to modify all the callers so that we get the same behavior everywhere (especially in RPC calls), because there is probably a nontrivial amount of work involved in changing a large ShardChunk into a ChunkView, especially if it's used in a place where we don't care about the formatting changes that that conversion makes

context: https://near.zulipchat.com/#narrow/stream/295558-core/topic/View.20Structs/near/437337420

This pulls in changes to master that are included in the branch marcelodg-stateless-forknet-mirror which is the one we're using to mirror transactions on statelessnet

[marcelo-gonzalez]

btw pytest/tests/tools/mirror/online_test.py is failing and I think it is probably because of the same thing that's causing single_shard_tracking.py to fail, where deploy contract txs are not making it on localnet for some reason

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 29.03226% with 44 lines in your changes missing coverage. Please review.

Project coverage is 71.78%. Comparing base (a32b0aa) to head (c3a7f91).

Files	Patch %	Lines
tools/mirror/src/lib.rs	0.00%	34 Missing :warning:
tools/mirror/src/online.rs	0.00%	5 Missing :warning:
chain/client/src/view_client_actor.rs	85.71%	1 Missing and 2 partials :warning:
tools/mirror/src/offline.rs	0.00%	2 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #11789 +/- ## ======================================= Coverage 71.78% 71.78% ======================================= Files 796 796 Lines 162924 162897 -27 Branches 162924 162897 -27 ======================================= - Hits 116948 116940 -8 + Misses 40921 40903 -18 + Partials 5055 5054 -1 ``` | [Flag](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | Coverage Δ | | |---|---|---| | [backward-compatibility](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (+<0.01%)` | :arrow_up: | | [db-migration](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.23% <0.00%> (+<0.01%)` | :arrow_up: | | [genesis-check](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.35% <0.00%> (+<0.01%)` | :arrow_up: | | [integration-tests](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `37.91% <0.00%> (+<0.01%)` | :arrow_up: | | [linux](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.30% <29.03%> (+<0.01%)` | :arrow_up: | | [linux-nightly](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `71.36% <29.03%> (+<0.01%)` | :arrow_up: | | [macos](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `54.58% <29.03%> (+0.03%)` | :arrow_up: | | [pytests](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.58% <0.00%> (+<0.01%)` | :arrow_up: | | [sanity-checks](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `1.38% <0.00%> (+<0.01%)` | :arrow_up: | | [unittests](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `66.22% <29.03%> (+<0.01%)` | :arrow_up: | | [upgradability](https://app.codecov.io/gh/near/nearcore/pull/11789/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near) | `0.28% <0.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=near#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[frol]

@marcelo-gonzalez Sorry for jumping in, I'd like to invite you to participate in the Race of Sloths. Just mention @race-of-sloths user in your github comment or PR description to join!

See how the flow works here: https://github.com/near/nearcore/pull/11778

[tayfunelmas]

I am not in favor of exposing a core data structure (ShardChunk) in the RPC interface. Can we make this an internal RPC (private and not available to others)? For example marking it with "INTERNAL" prefix similar to "EXPERIMENTAL" methods?

Or can we have a new RPC method for retrieving the missing parts, eg. a method for getting contract code by its hash?

[marcelo-gonzalez]

I am not in favor of exposing a core data structure (ShardChunk) in the RPC interface. Can we make this an internal RPC (private and not available to others)? For example marking it with "INTERNAL" prefix similar to "EXPERIMENTAL" methods?

Or can we have a new RPC method for retrieving the missing parts, eg. a method for getting contract code by its hash?

I am not in favor of exposing a core data structure (ShardChunk) in the RPC interface .. Or can we have a new RPC method for retrieving the missing parts

This doesn't actually have anything to do with the RPC interface. All of the RPC code is unchanged by this PR. Here what we're doing is just adding an extra request/response type to the ViewClientActor. Within the set of existing ViewClientActor request/response types, there are already many types that are "internal" in some way and are never exposed to any RPC method. See for example this one used by the indexer or this one used by the network code to generate state sync header responses. In this PR what we're doing is just adding yet another one of these, wich is going to be used just in the mirror code, not anywhere in the RPC code

And as for the "internal" nature of the ShardChunk type that we're returning here, I think there are a couple reasons why this is not really an issue in this PR. The first is that the caller of this new GetShardChunk view client request type that returns a ShardChunk already deals with this type. It's not like we're adding its use in a place where it wasn't previously used. The ShardChunk type is already a type that gets passed around all over the codebase. And secondly, even if we were adding a usage of ShardChunk where it didn't previously exist, there's not much of a meaningful practical difference between the ShardChunk and ChunkView types. When we convert from a ShardChunk to a ChunkView, we basically just copy over all the fields into a new struct definition. There's really nothing extra that gets exposed by the ShardChunk type (except of course for the deploy contract actions which are the reason for this PR. But that doesn't feel like an intentional hiding of "private" data, it's just something that was never needed), which is why I made the comment earlier that it would make sense to me to just get rid of this ChunkView type altogether, since I don't really see much point in copying all the fields of one type to just write them into a functionally identical struct (functionally identical from the point of view of the caller that just needs the chunk info)

But yea the important point is that there is nothing to do with RPC methods here

[tayfunelmas]

But yea the important point is that there is nothing to do with RPC methods here

Oh ok, thanks for the explanation. So the new handler is internal anyways.

[marcelo-gonzalez]

But yea the important point is that there is nothing to do with RPC methods here

Oh ok, thanks for the explanation. So the new handler is internal anyways.

Yeah, none of this is exposed anywhere to RPC callers (and if it were, I think it would still be fine as long as we make sure the return value of RPC methods is identical to what it always has been)

[tayfunelmas]

LGTM