Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add console checks for sct divergence #882

Merged
merged 1 commit into from
Apr 4, 2024
Merged

Conversation

grod220
Copy link
Collaborator

@grod220 grod220 commented Apr 4, 2024

Closes #878

vid_compressed.mp4

@@ -141,7 +148,7 @@ export class BlockProcessor implements BlockProcessorInterface {

private async syncAndStore() {
const fullSyncHeight = await this.indexedDb.getFullSyncHeight();
const startHeight = fullSyncHeight ? fullSyncHeight + 1n : 0n;
const startHeight = fullSyncHeight !== undefined ? fullSyncHeight + 1n : 0n; // Must compare to undefined as 0n is falsy
Copy link
Collaborator Author

@grod220 grod220 Apr 4, 2024

Choose a reason for hiding this comment

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

Found an interesting bug. After we sync block 0, we save full sync height as 0. However, 0 is a falsy value. That means if the block processor was reset between blocks 0 and 1000, we'd have a sct root divergence.

Screenshot 2024-04-04 at 12 03 38 PM

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice catch 👍
I wonder if this fixes the divergence that occurs on Henry's wallet (I suspect Henry didn't stop synchronization specifically between the 0 and 1000 block)

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I did make transactions in the first 1000 blocks, so if it paused to fetch full tx details, that could have been an opportunity for interruption...

@grod220 grod220 merged commit 69e02d5 into main Apr 4, 2024
6 checks passed
@grod220 grod220 deleted the sct-divergence-check branch April 4, 2024 12:27
Comment on lines +41 to +45
declare global {
// `var` required for global declaration (as let/const are block scoped)
// eslint-disable-next-line no-var
var ASSERT_ROOT_VALID: boolean | undefined;
}
Copy link
Member

Choose a reason for hiding this comment

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

@grod220 can we get some docs in the dev docs about the existence of this global and how you would set it and why?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good 👍
#891

@grod220 grod220 mentioned this pull request Apr 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Runtime configuration of SCT divergence checks in web extension
3 participants