Handlers refactoring

[GitGab19]

This PR refactors handlers inside roles_logic_sv2 crate.

Closes #1443

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 125 lines in your changes missing coverage. Please review.

Project coverage is 19.60%. Comparing base (e44e16f) to head (d6b4821).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...rotocols/v2/roles-logic-sv2/src/handlers/mining.rs	0.00%	47 Missing ⚠️
...v2/roles-logic-sv2/src/handlers/job_declaration.rs	0.00%	19 Missing ⚠️
...rotocols/v2/roles-logic-sv2/src/handlers/common.rs	0.00%	16 Missing ⚠️
protocols/v2/roles-logic-sv2/src/routing_logic.rs	0.00%	9 Missing ⚠️
roles/jd-client/src/lib/downstream.rs	0.00%	8 Missing ⚠️
roles/mining-proxy/src/lib/upstream_mining.rs	0.00%	7 Missing ⚠️
protocols/v2/roles-logic-sv2/src/parsers.rs	0.00%	5 Missing ⚠️
roles/jd-client/src/lib/upstream_sv2/upstream.rs	0.00%	3 Missing ⚠️
roles/translator/src/lib/upstream_sv2/upstream.rs	0.00%	3 Missing ⚠️
...s/v2/subprotocols/common-messages/src/reconnect.rs	0.00%	2 Missing ⚠️
... and 5 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1567      +/-   ##
==========================================
+ Coverage   19.31%   19.60%   +0.29%     
==========================================
  Files         131      132       +1     
  Lines        9298     9160     -138     
==========================================
  Hits         1796     1796              
+ Misses       7502     7364     -138     

Flag	Coverage Δ
binary_codec_sv2-coverage	0.00% <0.00%> (ø)
binary_sv2-coverage	9.00% <0.00%> (+0.40%)	⬆️
bip32_derivation-coverage	0.00% <ø> (ø)
buffer_sv2-coverage	37.68% <ø> (ø)
codec_sv2-coverage	0.03% <0.00%> (+<0.01%)	⬆️
common_messages_sv2-coverage	0.22% <0.00%> (+<0.01%)	⬆️
const_sv2-coverage	0.00% <0.00%> (ø)
error_handling-coverage	0.00% <ø> (ø)
framing_sv2-coverage	0.48% <0.00%> (+0.02%)	⬆️
jd_client-coverage	0.44% <0.00%> (ø)
jd_server-coverage	14.50% <ø> (ø)
job_declaration_sv2-coverage	0.00% <0.00%> (ø)
key-utils-coverage	3.61% <ø> (ø)
mining-coverage	4.06% <0.00%> (+0.20%)	⬆️
mining_device-coverage	0.00% <ø> (ø)
mining_proxy_sv2-coverage	0.97% <0.00%> (-0.01%)	⬇️
noise_sv2-coverage	7.65% <0.00%> (+0.35%)	⬆️
pool_sv2-coverage	2.67% <0.00%> (-0.01%)	⬇️
protocols	28.06% <0.00%> (+0.90%)	⬆️
roles	7.11% <0.00%> (-0.02%)	⬇️
roles_logic_sv2-coverage	14.04% <0.00%> (+0.47%)	⬆️
sv2_ffi-coverage	0.00% <0.00%> (ø)
template_distribution_sv2-coverage	0.00% <0.00%> (ø)
translator_sv2-coverage	10.17% <0.00%> (-0.01%)	⬇️
utils	36.39% <ø> (ø)
v1-coverage	3.91% <0.00%> (+0.16%)	⬆️

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.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[plebhash]

I was under the impression that we would also get some deeper simplifications on ParseMiningMessagesFromDownstream and ParseMiningMessagesFromUpstream with regards to Selector and Router generics

is that going to be done on future PRs?

or did you conclude that it's not possible for some reason?

[plebhash]

another relevant point related to the scope of this PR is the fact that for some reason, mining_device::Device crate is forced to add null implementations for traits that are clearly meant for Servers

stratum/roles/test-utils/mining-device/src/lib/mod.rs

Lines 345 to 388 in 079e823

	impl IsUpstream<(), NullDownstreamMiningSelector> for Device {
	fn get_version(&self) -> u16 {
	todo!()
	}
	fn get_flags(&self) -> u32 {
	todo!()
	}
	fn get_supported_protocols(&self) -> Vec<Protocol> {
	todo!()
	}
	fn get_id(&self) -> u32 {
	todo!()
	}
	fn get_mapper(&mut self) -> Option<&mut roles_logic_sv2::common_properties::RequestIdMapper> {
	todo!()
	}
	fn get_remote_selector(&mut self) -> &mut NullDownstreamMiningSelector {
	todo!()
	}
	}
	impl IsMiningUpstream<(), NullDownstreamMiningSelector> for Device {
	fn total_hash_rate(&self) -> u64 {
	todo!()
	}
	fn add_hash_rate(&mut self, _to_add: u64) {
	todo!()
	}
	fn get_opened_channels(
	&mut self,
	) -> &mut Vec<roles_logic_sv2::common_properties::UpstreamChannel> {
	todo!()
	}
	fn update_channels(&mut self, _: roles_logic_sv2::common_properties::UpstreamChannel) {
	todo!()
	}
	}

[plebhash]

related to my comments above, here's how I visualize the ideal ParseMiningMessagesFromUpstream and ParseMiningMessagesFromDownstream:

- pub trait ParseMiningMessagesFromUpstream<
-    Down: IsMiningDownstream + D,
-    Selector: DownstreamMiningSelector<Down> + D,
-    Router: MiningRouter<Down, Self, Selector>,
- > where
-     Self: IsMiningUpstream<Down, Selector> + Sized + D,
+ pub trait ParseMiningMessagesFromUpstream
+ where
+    Self: Sized,

- pub trait ParseMiningMessagesFromDownstream<
-    Up: IsMiningUpstream<Self, Selector> + D,
-     Selector: DownstreamMiningSelector<Self> + D,
-    Router: MiningRouter<Self, Up, Selector>,
- > where
-    Self: IsMiningDownstream + Sized + D,
+ pub trait ParseMiningMessagesFromDownstream
+ where
+    Self: Sized,

I feel all those generics and trait bounds are over-engineered, bringing a lot of unnecessary complexity without any clear benefit

if we compare them with their JDP and TDP equivalents, we see that those are much simpler

[GitGab19]

I was under the impression that we would also get some deeper simplifications on ParseMiningMessagesFromDownstream and ParseMiningMessagesFromUpstream with regards to Selector and Router generics

is that going to be done on future PRs?

or did you conclude that it's not possible for some reason?

Selectors and Router generics are gonna be removed in a follow-up PR specifically tied for refactoring those modules.

I have it almost ready locally, it's built on top of this PR and will be pushed soon

[plebhash]

I was under the impression that we would also get some deeper simplifications on ParseMiningMessagesFromDownstream and ParseMiningMessagesFromUpstream with regards to Selector and Router generics
is that going to be done on future PRs?
or did you conclude that it's not possible for some reason?

Selectors and Router generics are gonna be removed in a follow-up PR specifically tied for refactoring those modules.

I have it almost ready locally, it's built on top of this PR and will be pushed soon

that's great, I suspected that could be the case

but it still leaves the questions about IsMiningUpstream and IsMiningDownstream open

perhaps that could also be addressed in a follow-up PR, but I just want to make sure we don't wrap up the refactorings on handlers module without addressing them, as the point I mentioned about the Sv2 CPU miner indicates there's something very wrong with those trait bounds

[GitGab19]

Thanks for putting emphasis on this, it's very important. I'm gonna tackle it in the next PR, promise! :)