Daemon RPC: high start_height ok in /getblocks.bin [v0.18]

[j-berman]

This PR: If the client requests a start_height > current chain tip, then the daemon returns a successful response that includes the chain's height.

Current behavior on release is to ban a client that requests such a height.

This PR's behavior is useful both in this FCMP++ integration PR's approach to syncing that simplifies wallet refresh while minimizing data downloaded by the client, and in the async scanner.

#9382
#10025

[spirobel]

here is an alternative to this that would require just two line changes:
remove the two add_host_fail lines that got introduced in this crazy commit:
j-berman@a182df2

that means that behavior of the RPC stays the same, but it doesn't block clients anymore.

the benefit of this solution is that it does not add technical debt in the form of yet another option and removes unnecessary code. It would also enable the deletion of the add_host_fail method.

In essence the solution is to just revert the commit that introduced this weird rpc client banning behavior on http 500.
(it is still odd to not just return an empty array instead of an http 500, but it is what it is.)

[j-berman]

This idea was also aiming to solve for the case that the client still wants to know the daemon's height without another round trip to the daemon (not just get rid of the ban)

[j-berman]

In theory it's not strictly necessary for the case described here, since there should still be one successful response (that we can use to explain the other RPC failures). But a success response that includes the height does make the response simpler to manage.

[spirobel]

a success response that includes the height does make the response simpler to manage.

true.
Lets make the success response the default behavior and not introduce this line:

KV_SERIALIZE_OPT(high_height_ok, false) // default false maintains backwards compatibility for clients that relied on failure on high height 

It is good to be conservative, but at the same time: what is the worst case we are trying to avoid? In this case it would be that syncing might get stuck? But the success response will contain a new daemon height, it will pick that up and then sync towards that.

The other part is removing add_host_fail. But maybe that can be a separate PR.

[j-berman]

what is the worst case we are trying to avoid?

An example: a dev using the current API might expect a failure response when they provide too high of a height to e.g. know when to stop fetching blocks. Their software could end up infinite looping, or totally breaking, if the endpoint returns a success response instead.

Maybe we can claim that would be the incorrect way of knowing when to stop fetching blocks. But I can imagine someone being peeved with their software breaking due to a change to the endpoint.

If we were to make the breaking change and bump the RPC major version, then older clients would not be able to use the updated daemon. Thus, I leaned conservative on this with the (obviously unfortunate) new request field.

[j-berman]

Actuaaaaally, since current behavior is to ban clients requesting a high height, there's no way anyone is justifiably relying on that behavior...

I think we're justified in getting rid of this high_height_ok request field. I'll get rid of it here and on master too.