Page MenuHomePhabricator

don't allow negative nblocks for getnetworkhashps
AbandonedPublic

Authored by PiRK on Sep 17 2021, 15:34.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Summary

We no longer have a concept of "since last difficulty adjustement", now
that the difficulty is adjusted for each new block.
Remove that option from the getnetworkhashps RPC and throw an error.

Test Plan

ninja all check-all

Diff Detail

Event Timeline

PiRK requested review of this revision.Sep 17 2021, 15:34
Fabien requested changes to this revision.Sep 19 2021, 05:34
Fabien added a subscriber: Fabien.

This is incomplete: you make the feature unreachable, but don't remove it. This results in dead code that can is hard to understand because there is no traceable legacy use case anymore.

This revision now requires changes to proceed.Sep 19 2021, 05:34

Make lookup unsigned, in GetNetworkHashPS, make sure it is never less than 1

Fabien requested changes to this revision.Sep 20 2021, 08:03

There are 2 issues here:

  • You also changed the behavior when nblock = 0, with no test (and yes the RPC doc was wrong previously)
  • Since you're changing the behavior of the RPC, and this feature is potentially used by miners (I can imagine switching miners using that), this needs to go through a deprecation phase first.
src/rpc/mining.cpp
59

That seems redundant with the above check.

68

lookup is no size_t

This revision now requires changes to proceed.Sep 20 2021, 08:03

Need to figure out a deprecation plan for this