near / rainbow-bridge-sol

10 stars 8 forks source link

Change the state machine used for processing new blocks #20

Closed abacabadabacaba closed 4 years ago

abacabadabacaba commented 4 years ago

Now a newly added block that is not yet trusted is stored in untrustedHead. After the challenge period is over, it can be moved to head. Fixes #13, #15.

abacabadabacaba commented 4 years ago

I fixed verify_near_headers, but it still won't pass cli tests because there is new constructor parameter.

abacabadabacaba commented 4 years ago

This logic makes contract more complex by adding an additional public method. Instead we can make head and backupHead private and add an additional method getTrustedHead() which returns either head or backupHead depending on whether validAfter has passed.

I think that the logic is now less complex. There is head which is trusted. A new method is needed to move untrustedHead to head when the challenge period is over without pushing a new block. This also makes this block available to blockHashes and blockMerkleRoots. It is possible to add an extra method, but it won't make things less complex, because there should be still a way to commit a block so that NearProver can use it (it uses blockMerkleRoots).

Or I can just make commitBlock internal, so a block will only become available when another block is submitted on top of it.

Overall, this code needs a lot of refactoring, but for now I try not to change it too much.

MaksymZavershynskyi commented 4 years ago

This also makes this block available to blockHashes and blockMerkleRoots. It is possible to add an extra method, but it won't make things less complex, because there should be still a way to commit a block so that NearProver can use it (it uses blockMerkleRoots).

We could add trustedMerkleRoots method that filter the last merkle root if the header has not passed the challenge period yet. By adding commitBlock we are delegating complexity to the contract user who now need to call an extra method before being able to use this contract.

Or I can just make commitBlock internal, so a block will only become available when another block is submitted on top of it.

This will make header that passed challenge period not useful until the next header is submitted. Ideally, we want header to be useful as soon as it passes the challenge period.

abacabadabacaba commented 4 years ago

We could add trustedMerkleRoots method that filter the last merkle root if the header has not passed the challenge period yet.

That's not a good API design. "Trusted" should be the default (and indeed the only) option.

Anyway, I changed the API to return the last block as soon as the challenge period is over.

abacabadabacaba commented 4 years ago

@nearmax I addressed the comments and also fixed another bug, is everything OK now? Will make a PR for near-bridge-cli and -lib a bit later. I can't merge this PR because of CI.

abacabadabacaba commented 4 years ago

Created pull requests: near/rainbow-bridge-cli#348, near/rainbow-bridge-lib#14.

MaksymZavershynskyi commented 4 years ago

I have tested this change locally, so I am going to merge it and publish the package.