ValidateAppProtocolVersion should not check extra data

[Atralupus]

We implemented a separate updater in the launcher for each project(player, launcher...) - planetarium/9c-launcher#1731

It's almost done, and this updater is dependent on AppProtocolVersion extra data - planetarium/9c-launcher#1744
AppProtocolVersion Shema looks like below

APV(100291)
player: "1/3f26a4c7a2f4fecbca8fe45a5969b85b5b3a189a"
launcher: "1/39aa07191148615acdb6dd48e9d05c7446f3d6b7"

Now, if only the player changes exist, then only the player part will be changed. not increase AppProtocolVersion version

But Currently ValidateAppProtocolVersion method in MessageValidator is check AppProtocolVersion equality using this code

libplanet/Libplanet.Net/AppProtocolVersion.cs

Line 239 in d05c843

public bool Equals(AppProtocolVersion other) =>

We want the ValidateAppProtocolVersion method should not to check extra data

[Atralupus]

I tried to solve this issue in https://github.com/Atralupus/libplanet/tree/feat/except-extra-camparer this branch
but I've faced Sign method, Sign using extra data for creating signature

[dahlia]

Although Nine Chronicles wants Libplanet to consider any APVs signed by trusted parties even if their extra data differ from the current node's one, it's still unsure if it's generally good practice for every game/app made with Libplanet. IMHO it should be configurable.

In fact, there was an option to accept or ignore different APVs signed by trusted parties, but it was removed for the simplicity (see also: #1885). It might be better to revert the removal (or provide a better API for it).

[Atralupus]

That Issue seems to exist with the option to accept different APV, not check APV's extra part.
But, This issue wants to check extra data specifically

As you mentioned, I would provide a better API to check extra
Also, I will put effort into this function can be configurable

[greymistcube]

Although Nine Chronicles wants Libplanet to consider any APVs signed by trusted parties even if their extra data differ from the current node's one, it's still unsure if it's generally good practice for every game/app made with Libplanet. IMHO it should be configurable.

I don't quite agree with this sentiment. Although the intention of APVs as they are used by us may be a bit more clear to us, I'd say providing an option what to do with it makes what APVs are for even more vague. To an outsider, it may look like "Here is this thing called APV, do whatever you want with it". At that point, we might as well off-load APV validation and handling to the application and use something like IsAppProtocolVersionAcceptable and/or AppProtocolVersionHandler delegates. 😶

Also the removed option is too "coarse" for the issue at hand as @Atralupus points out. See the comment below. 😗

[greymistcube]

I was a bit confused for a while and had to go over the context several times to understand what is going on (I think?). 🙄

Current Implementation

APV is structured to have three parts:

• Version number
• Signer identity
• Extra data

As it stands, APV equality is defined as "exact equality" where all three of the above must match. If a node receives a message with a different APV, two things "happen":

• The receiving node triggers the DifferentAppProtocolVersionEncountered delegate provided by the application, namely NineChronicles for our case, in order to let the application handle whatever that is needed to be done.
• Libplanet itself does nothing with the received message with a different APV.

Intention of APV

Although not specified explicitly in AppProtocolVersion, I think it'd be safe to assume "different" APV, as in different version number, i.e. Version, not as in not equal as defined above, is supposed to mean the nodes would not be compatible due to how Libplanet is designed¹. Essentially, a higher APV signals that for the receiving node execution results are (possibly) going to be different from the node sending out the message. Under normal circumstances, we'd want the lower number APV node to trigger auto-update the client so that it can properly sync up to the nodes with a higher APV.

Wanted Feature

From the application side, although I'm not directly involved, I'm guessing the reason for the original post and the diagram by @Atralupus is to have launcher update cycle decoupled with the player update cycle. This can be interpreted several ways:

• Trigger launcher update without full deployment with an APV bump.
• Have launcher update while the player is continuously running.²

In either case, some kind of callback to the external application must be triggered. As for game clients, if we go with the first option, this isn't much of an issue as DifferentAppProtocolVersionEncountered will be triggered as it stands, and "distributed nodes" for players can be forcefully updated if needed.

However, the problem is further complicated by NineChronicles.Headless nodes, as these are independent from the launcher client, as we'd want, or at least that is how I understand it, the deployed cluster intact.

Premise

I think it'd be a good idea to define basic premises for APVs to make this discussion more concise. Let's define some terms first (by borrowing from math).

• Two APVs are said to be equal if Versions, Signers, and Extras are all the same.
• Two APVs are said to be strongly equivalent if Versions and Signers are the same.³
• Two APVs are said to be weakly equivalent if Versions are the same.

Consider the following premise:

• Any two nodes with weakly equivalent APVs are compatible.

If we accept this as part of the spec for Libplanet, then I'd say it makes sense for any node to accept all messages with weakly equivalent APVs as this should guarantee proper consensus between nodes.⁴

Then the issue at hand seems to be that the calling conditions for DifferentAppProtocolVersion and message accepting conditions are treated as the same when they should not be.

Footnotes

1. The library depending on externally provided assembly to execute game actions. ↩

2. I'm not sure how this would be possible as a client's backend would have to use the updated APV. ↩

3. Although this hasn't come into play yet really, but it could be used as a fail-safe mechanism. Other than that, in order to truly trust a different signer, we'd need an air-tight specification of Libplanet, whatever application that might be using Libplanet, possibly a test suite to pass. I don't know how realistic this scenario may be in any foreseeable future. 🙄 ↩

4. The validity of a received APV is an entirely different issue. This must be checked separately. ↩

[Atralupus]

We discussed and draw out the results
Just append only check APV version

libplanet/Libplanet.Net/Messages/MessageValidator.cs

Line 157 in 71e204f

throw new DifferentAppProtocolVersionException(

above this line

Currently AppProtocolVersion values Version not other data (FYI @greymistcube 's Intention of APV)

[riemannulus]

I guess this discussion is still available and AFAIK @Atralupus and @greymistcube are discussed on discord about this. Have you any issues or ideas about this discussion?