JSON-RPC: Add finalized and safe blocks

[mkalinin]

Adds finalized value to the block tags enum.

Pre-Merge response

There are three options:

• Refer to a block that is likely to stay in the PoW canonical chain forever, i.e. a block which is e.g. 128 blocks behind the head
• Respond with error
• Respond with genesis block

I am slightly inclined towards the third option. It gives users a graceful option to check whether the Merge has happened or not, i.e. checking if finalized.number > earliest.number.

Whatever option we choose it needs to be specified somewhere. Probably in the description field to this enum.

Safe block

Another couple of values that are considered for addition to this enum is safe and unsafe. It worth noting that if these values and the corresponding functionality will be added after the Merge then unsafe tag will be used as an alias to the latest to avoid backwards incompatibility.

Description

I've been playing with the way of adding a "description" field to this enum in the openrpc playground. To be honest, it looks ugly as it seems there is no way to describe each value separately and there is also no way to include line breaks into the description. So, whenever we will have to utilize description it will be a sequence of sentences without breaks.

cc @timbeiko @lightclient @djrtwo @MicahZoltu @MariusVanDerWijden

[mkalinin]

"safe" and "unsafe", and the description are added. This PR is ready for review now. I will create a separate one to add "justified".

[MicahZoltu]

I still have comments, but none that should block this from being merged.

[mkalinin]

I have taken descriptions suggested by @MicahZoltu and modified them a bit in this document https://hackmd.io/T8LBBP6HSLqGFWM8ikQakA. Let's agree on the texts in hackmd and then just move it into the schema. I feel it should be more convenient to do

[lightclient]

Theoretically, the description field should support Github markdown.

[lightclient]

unless there is an active ongoing attack

Is an attack really only the possibility of reorging a safe block?

high probability of being reorged out

I think it is actually more accurate to say that an unsafe block has very little weight behind it and may be reorged rather cheaply?

currently an alias for unsafe or safe

We should probably make a decision on this before merging?

[mkalinin]

@lightclient Some of your comments/suggestion are addressed in https://hackmd.io/T8LBBP6HSLqGFWM8ikQakA.

Apparently OpenRPC playground doesn't support Markdown which is odd

[MicahZoltu]

Theoretically, the description field should support Github markdown.

I believe the problem is that JSON doesn't allow newlines, and Markdown doesn't have a mechanism for inserting newlines other than with the newline character (just as it doesn't have a mechanism for expressing an a other than by using the a character). This means that there is functionally no way to get a newline inserted, even though it would be supported if you could get it in there.

[MicahZoltu]

We should probably make a decision on this before merging?

It will alias to unsafe for now, but we may move it to safe in the future.

[djrtwo]

I'm weakly against removing latest from the enum at this point given we don't have a good safe algorithm that follows near the head. thus steering users to use safe for most purposes doesn't yet capture the intent -- it trails by 6 to 10 minutes in current "justified" algorithm.

My preference would be to deprecate latest once we have a good (near head in normal case) safe algorithm. Until then, I feel that latest has high semantic value.

high probability of being reorged out

also, I think this is a bit extreme of language when we have the huge spread between safe and unsafe today. a block a few blocks deep in most cases does not have a high probability of being re-org'd out.

If we move forward as is, I'll be strongly against fully removing latest until the safe/unsafe alg disparity is fixed

[djrtwo]

Is an attack really only the possibility of reorging a safe block?

@lightclient in the current algorithm which only uses "justified", then yes -- an attack or a bug.

The assumptions we'll likely use in a future "safe" algorithm will have a couple more assumptions -- 50%+ honest (non-attacking) and some synchrony bound. So it's not just an attack, a high asynchrony (depending on the assumption in the algorithm) could also re-org blocks

[mkalinin]

I have updated the description according to the doc.

My preference would be to deprecate latest once we have a good (near head in normal case) safe algorithm. Until then, I feel that latest has high semantic value.

I concur. latest is currently marked as deprecated and it is said that it will be removed in the future -- i think this is pretty visible announcement taking in account that users will read the description because of new tags.

[mkalinin]

The latest value is brought back, the description starts from "latest: DEPRECATED" -- this way the deprecation should be noticeable by users

[djrtwo]

I've been thinking about this, and I actually don't think unsafe is correct/valuable word here even in the long run (or especially in the long run).

In the event that we define a safe head algorithm that more often than not is latest (is the head), then in such a case, "safe" and "unsafe" would often return the same thing. This is semantically unsound and confusion -- something that is safe cannot simultaneously be unsafe.

Thus I think it is actually best to keep "latest" -- a word that has strong semantic meaning and (I argue) will always be of value to some set of users. And add "safe" (an algorithm we hope to improve) and 'finalized"

That is -- do not deprecate "latest" and do not add "unsafe"

[dankrad]

I don't think I agree with this interpretation of the semantics. "safe" and "unsafe" are not referring to a block, but instead to the way that block was derived. Both methods can indeed come to the same block, but this is not a contradiction.

[mkalinin]

I don't think I agree with this interpretation of the semantics. "safe" and "unsafe" are not referring to a block, but instead to the way that block was derived. Both methods can indeed come to the same block, but this is not a contradiction.

I don't think that users associate these tags with the way of obtaining the blocks, it could be too involving for an average user. These tags will highly likely be associated with the blocks itself, thus, safe and unsafe referring to the same block is gonna be a source of confusion.

[djrtwo]

Agreed that you can interpret these as "the algorithm used to derive" but as Mikhail mentioned, I think it is incredibly unlikely that normal users (and even sophisticated ones at first glance!) would assume and properly interpret this.

[MicahZoltu]

I think one could make the same argument that users won't think through or even notice in most cases that "safe and unsafe" are the same. They'll likely try to understand at a very high level what each means, pick one, and query it (using whatever result it returns).

[mkalinin]

I've added

"Before the merge transition is finalized any call querying over finalized or safe block MUST be responded with error"

to the end of description -- looks ugly but this seems like the best we can do. I would rather specify an exact error code and message but apparently there is no unified list of errors for JSON-RPC and it may vary depending on client which means that whatever code we pick up we're risking to get into a conflict with already existing one or the one that will be introduced later on. If anyone have any ideas on figuring out the code we may use for this purpose I am keen to discuss

[MicahZoltu]

I recommend just picking an error code and message that seems reasonable and proposing it. Tell the execution clients, and if they find it overlaps with something they have we can change it. I think having no standard error code/message is far worse than having a conflicting error code/message.

[mkalinin]

I recommend just picking an error code and message that seems reasonable and proposing it. Tell the execution clients, and if they find it overlaps with something they have we can change it. I think having no standard error code/message is far worse than having a conflicting error code/message.

I picked up arbitrarily large custom error code. It should not conflict with already existing ones, and if we hit this code at some point in time we should be pretty far from the Merge which should preclude potential error code conflicts.

[MicahZoltu]

As long as it follows the constraints here, I don't care what you pick. 😁
https://www.jsonrpc.org/specification#error_object

[mkalinin]

As long as it follows the constraints here, I don't care what you pick. 😁
https://www.jsonrpc.org/specification#error_object

It's set to -32100 which is within the range of application defined errors

[MicahZoltu]

I believe application defined errors are "everything outside of [-32000,-32768]". -32100 would be for "server defined errors" (I don't know what that means).

-32000 to -32099 | Server error | Reserved for implementation-defined server-errors.
The remainder of the space is available for application defined errors.

[mkalinin]

I believe application defined errors are "everything outside of [-32000,-32768]". -32100 would be for "server defined errors" (I don't know what that means).

-32000 to -32099 | Server error | Reserved for implementation-defined server-errors.
The remainder of the space is available for application defined errors.

Yes, you're right. And it looks like we have messed up with this range in Engine API as well

[mkalinin]

I've replaced unsafe with latest and removed DEPRECATED note from the latter. Please, take a look once again and if everything is OK let's get it merged.

[MicahZoltu]

Just some grammar stuff and a minor addition to the definition of latest.

[mkalinin]

Just some grammar stuff and a minor addition to the definition of latest.

Applied