defer channels_sv2::client share acceptance to upstream acknowledgement

[plebhash]

close #2089

breaking change on channels_sv2, bumps major version

we replace channels_sv2::client::ShareAccounting::update_share_accounting with:

• track_validated_share (still called from validate_share)
• on_share_acknowledgement (called from the on_share_acknowledgement introduced into the channels layer)

application layer calls on_share_acknowledgement on channels every time a SubmitSharesSuccess arrives

companion stratum-mining/sv2-apps#283

[Shourya742]

Rest of the changes look good

[Shourya742]

Yeah, we can make new_share_sum a u64, based on the comments in the sv2-apps PR: stratum-mining/sv2-apps#283 (comment) . There’s no real need to keep it as an f64, it looks like an inaccuracy that slipped through during the initial iteration.

[GitGab19]

IIRC, the new_shares_sum represent the amount of work acknowledged by the server in a specific SubmitSharesSuccess, that's why it's a f64.

But I might be wrong here.

[plebhash]

I remember seeing work being reported as floating point on Braiins UI, and also contemplating the fact that strictly speaking, work is a unit of difficulty, and therefore it can (and maybe should) be tracked as floating point numbers (like rust bitcoin does)

this is specially true when we're operating at diff ranges 0 < diff < 1, where we have no relevant data unless we're working with floating point numbers

but as @Shourya742 correctly points out, this was kind of a failed attempt at retaining granular data, because SubmitSharesSuccess.new_shares_sum is U64, which forces everyone to truncate and makes an internal representation based on f64 pretty much useless

FWIW there's some extra context on stratum-mining/sv2-spec#126

---

with that said, I'd like to avoid expanding the scope of this PR to also cover for this, because that's not the problem #2089 we're trying to solve

created #2092 to keep track of it

[Shourya742]

Acc to spec, its the sum of share acknowledged in the batch,

[Shourya742]

I was not aware of that PR. Thanks for mentioning that.

[plebhash]

Acc to spec, its the sum of share acknowledged in the batch,

no, you're confusing it with new_submits_accepted_count

please see stratum-mining/sv2-spec#126, I drafted this while interacting with Braiins

[GitGab19]

ACK, just a minor thing.

[GitGab19]

we need to bump PATCH here to publish it on our next release

[plebhash]

bebe82f

[Shourya742]

Green from me, once Gitgab's comments are addressed.