Skip to content
This repository has been archived by the owner on Oct 20, 2020. It is now read-only.

Add replace-duration parameter for NearClient #14

Merged
merged 4 commits into from
Oct 6, 2020

Conversation

abacabadabacaba
Copy link
Contributor

No description provided.

@abacabadabacaba abacabadabacaba force-pushed the add-replace-duration branch 4 times, most recently from 5c42e0f to 65afa41 Compare September 20, 2020 23:05
console.log(
`Near2EthClient block is at ${clientBlockHeight.toString()} which is further than the needed block ${outcomeBlockHeight.toString()}`
)
if (clientBlockHeight > outcomeBlockHeight) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does JS int okay here? it's up to 2^52, bigger than that will become imprecise

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we won't reach block height of more than 253≅9·1015 any time soon.

try {
result = await this.clientContract.methods.checkBlockProducerSignatureInHead(i).call()
} catch (e) {
console.log(`Error, continuing.`)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to not attempt to call checkBlockProducerSignatureInHead when signature is missing, because currently we cannot differentiate between transient errors, e.g. RPC-related stuff and error that occurs when we try to call checkBlockProducerSignatureInHead on missing signature.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current logic should work fine in both cases. Suppose that it will be possible to differentiate between those cases, will watchdog's behavior be different?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now sleeps timeDelta before revalidating the block. Ideally this should be parameter (currently when I run on gcloud I patch this code for timeDelta to be 300 because otherwise we would exceed infura quota). If user launches watchdog with timeDelta that is comparable to challenge period then watchdog can accidentally skip invalid signature because of some RPC error and wait to retry for timeDelta, which might be sufficient for bad header to be accepted. If we can differentiate between these two errors then we can retry signature check until RPC succeeds and only then sleep for timeDelta.

Also, currently this will print lots of spam logs, because testnet headers have routinely skipped signatures. So right now watchdog will be printing several Error, continuing every timeDelta seconds, which might be misinterpreted by the users.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the delay configurable, so this shouldn't be an issue anymore.

Copy link

@MaksymZavershynskyi MaksymZavershynskyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, assuming the comment is taken in consideration.

@abacabadabacaba abacabadabacaba force-pushed the add-replace-duration branch 2 times, most recently from 3351bc1 to c198303 Compare September 23, 2020 20:57
@MaksymZavershynskyi
Copy link

I have tested the change locally. I am now going to force merge it.

@MaksymZavershynskyi MaksymZavershynskyi merged commit 7cfa586 into master Oct 6, 2020
@abacabadabacaba abacabadabacaba deleted the add-replace-duration branch October 6, 2020 10:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants