Consider reduce reliance on web::Data/dynamic typing in rosettarpc

[matklad]

Might do something like

.app_data(web::Data::from(RosettaCtx {
 genesis,
 client_addr,
 view_client_addr,
 }))

instead of

.app_data(web::Data::from(genesis.clone()))
 .app_data(web::Data::new(client_addr.clone()))
 .app_data(web::Data::new(view_client_addr.clone()))

to lessen our dependency on dynamic typing here

Originally posted by @matklad in [ [https://github.com/near/nearcore/issues/6846#issuecomment-1132865770](https://github.com/near/nearcore/issues/6846#issuecomment-1132865770_) | https://github.com/near/nearcore/issues/6846#issuecomment-1132865770]

[mina86]

This would give check_network_identifier a place to live.

The counter argument is that not all handlers use all of those objects and having genesis field means we’ll have an Arc inside of an Arc.

PS. Rosetta RPC is actually Integrations team.

[matklad]

I don't think Arc is necessary:

struct RosettaCtx {
  genesis: Genesis
}

would work, no?

The counter argument is that not all handlers use all of those objects

They can still just not read the irrelevant fields :) And physically the situation would be quite isomorphic (the top-level handler/router in actix would probably still hold all the context), just with less dynamic-typing.

[matklad]

(added a brand-new T-integrations label, though TIL that such team exists)

[mina86]

Genesis is possibly a huge structure (it includes genesis records) so we don’t want to clone it into a new structure.

[matklad]

Ah, sorry, you are right, I misunderstood the way it works today, there's impl From<Arc<T>> for Data<T>, not impl From<T> for Data<T> (and that's why there's a weak guideline for spelling Arc::clone(&thing) rather that thing.clone()).

Still, I wouldn't worry about double indirection here, seems not important at this level of abstrction.

[mina86]

Double indirection is just a minor issue. The problem is that Genesis is potentially a large object. It’s small on mainnet so we could just shrug it off but on testnet the genesis records are non-trivial amount of data.

[matklad]

And

struct RosettaCtx {
  genesis: Arc<Genesis>
}

should fix that?

[mina86]

Right, yes, you’re right.

[firatNEAR]

I will look into this but is this a blocker for https://github.com/near/nearcore/pull/6846 or an enhancement on top of it?

[matklad]

Not a blocker at all -- just a small cleanup

[mina86]

Genesis is possibly a huge structure (it includes genesis records) so we don’t want to clone it into a new structure.

~By the way, after some further investigation, turns out we’re currently cloning genesis anyway:~

nearcore/src/lib.rs:306:        rpc_servers.extend(near_jsonrpc::start_http(
nearcore/src/lib.rs-307-            rpc_config,
nearcore/src/lib.rs-308-            config.genesis.config.clone(),
nearcore/src/lib.rs-309-            client_actor.clone(),
nearcore/src/lib.rs-310-            view_client.clone(),
nearcore/src/lib.rs-311-        ));

nearcore/src/lib.rs:318:            start_rosetta_rpc(
nearcore/src/lib.rs-319-                rosetta_rpc_config,
nearcore/src/lib.rs-320-                Arc::new(config.genesis.clone()),
nearcore/src/lib.rs-321-                client_actor.clone(),
nearcore/src/lib.rs-322-                view_client.clone(),
nearcore/src/lib.rs-323-            ),

~What we might try doing is introducing RPCContext which would be shared between JSON and Rosetta and then we could move genesis there only once.~

Actually, nvm, I’ve misread the code. We’re cloning config for JSON and genesis for Rosetta and with #7384 we will move genesis rather than cloning.

[matklad]

Yeah, traced the same thoughts! What feels odd to me is that we do manage to move Genesis into rosetta -- like, if everything else works fine without genesis, why rosetta of all things requires it?

:thinking: even if we don't clone genesis, it feels wrong that we keep it in memory only for rosetta.

[mina86]

Rosetta needs access to the genesis records. When someone fetches genesis block Rosetta needs to go through all the genesis records and translate them into Rosetta transactions. convert_genesis_records_to_transaction is the relevant piece of code. It is possible that this could be rewritten somehow. The records are in the database after all I think so Rosestta could read them from there maybe?