fix(network): register record shall not be verified by entire content

[maqi]

Description

For register record, the get_verification against the entire record value will have a high chance of failure, given the nature of the update_history MerkleReg held by the register.
For register, we can only carry out verification against root value, i.e. the final updated value

Related Issue

#2077

Type of Change

Please mark the types of changes made in this pull request.

• Bug fix (non-breaking change which fixes an issue)

[joshuef]

Would be good to get a test to verify this in some fashion. Not sure how best honestly...

And I'm wondering on the impacts here... (not sure how much of this would fall into the scope of this PR, but worth considering.)

During replication... do we fetch registers still with this setup? (I see it doesnt really touch flows there). But say, I have v1, and v2 exists... Would we still fetch + merge?

Is this purely on the client side saying that we return registers if it's the same address basically... but then how do we verify edits/updates?

And if there are updates / divergence, will the GET return all variations or just one at the moment? (Or will it handle merge if possible before returning?

[maqi]

get a test to verify this in some fashion

There is storage_payment_register_creation_succeeds test, which involves multiple ops mutation, publish and fetch verification.
Which shall cover this already ?
However, yeah, with the limited size of the CI local testnet, to fully simulate register with different update history is really tricky.

During replication... do we fetch registers still with this setup?

as explained at https://github.com/maidsafe/safe_network/pull/2103/files#diff-c302643eb99427e41bc3ae0fe33cdf9b841a8d55593fc5f20e132674ecfeb603R78
replication fetch doesn't have local target to verify with, hence the is_register flag can be set as false.

But say, I have v1, and v2 exists... Would we still fetch + merge?

that's a different question. And the answer is yes, we do fetch + merge
For register, during replication, after got it, eventually the validate_and_store_register function will be called.
which internally will get the local register merged with the fetched register

how do we verify edits/updates?

the root value, i.e. the final updated value, will still be verified against the target.
as at https://github.com/maidsafe/safe_network/pull/2103/files#diff-985b0f4f68bd09d2bcb4c2da67ab65c5c68d38d8aac388691e361a295bc8ae4bR169

And if there are updates / divergence, will the GET return all variations or just one at the moment? (Or will it handle merge if possible before returning?

After the merge of PR 2061, the fetched register will be the merged result of all copies from the closest nodes to the target.

[happybeing]

Supporting developers using these APIs is a good way to test this and much else. So long as the plan remains to replace the native token with a blockchain token, I will not help with that but am planning to keep awe working and people using that will in effect be testing things, at least until the native token is ditched.

You could also use awe yourselves, and may find the 'inspect-register' command that is part of the awe CLI is helpful in debugging any issues.

[joshuef]

Supporting developers using these APIs is a good way to test this and much else.

Yeh that'll be the true final test, but just wanting to make sure we have an automated CI step that gets a minimum case in here.

[joshuef]

the root value, i.e. the final updated value, will still be verified against the target.
as at #2103 (files)

Aha, okay so we're comparing the final computed value only.

To me this is not the root value... or rather, the root to me implies the first entry. Whereas here we mean the cumulative final hash after all merging...

If that's the case, i'd suggest renaming the idea of the base register to be merged_register or something more clear?

[maqi]

To me this is not the root value... or rather, the root to me implies the first entry. Whereas here we mean the cumulative final hash after all merging...

The crdts::MerkleReg document defines the roots as:

The MerkleReg is a Register CRDT that uses the Merkle DAG structure to track the current value(s) held by this register. The roots of the Merkle DAG are the current concurrent values.

Here just try to follow that definition.

If that's the case, i'd suggest renaming the idea of the base register to be merged_register or something more clear?

base_register is a member field of the SignedRegister.
it has been used like this before this PR, and I think is not related to the work ?

[joshuef]

Okay fair enough! @maqi , thanks for clarifying 👍