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
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 28 additions & 8 deletions packages/query/src/block-processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ import { getAssetId } from '@penumbra-zone/getters/src/metadata';
import { STAKING_TOKEN_METADATA } from '@penumbra-zone/constants/src/assets';
import { toDecimalExchangeRate } from '@penumbra-zone/types/src/amount';
import { ValidatorInfoResponse } from '@buf/penumbra-zone_penumbra.bufbuild_es/penumbra/core/component/stake/v1/stake_pb';
import { uint8ArrayToHex } from '@penumbra-zone/types/src/hex';

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;
}
Comment on lines +41 to +45
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


interface QueryClientProps {
querier: RootQuerier;
Expand Down Expand Up @@ -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...

let latestKnownBlockHeight = await this.querier.tendermint.latestBlockHeight();

if (startHeight === 0n) {
Expand Down Expand Up @@ -279,6 +286,10 @@ export class BlockProcessor implements BlockProcessorInterface {
if (isLastBlockOfEpoch) {
await this.handleEpochTransition(compactBlock.height, latestKnownBlockHeight);
}

if (globalThis.ASSERT_ROOT_VALID) {
await this.assertRootValid(compactBlock.height);
}
}
}

Expand Down Expand Up @@ -384,17 +395,26 @@ export class BlockProcessor implements BlockProcessorInterface {
}
}

// Compares the locally stored, filtered TCT root with the actual one on chain. They should match.
// This is expensive to do every block, so should only be done in development for debugging purposes.
// Recommend putting it alongside a flush (when flushReasons are triggered).
// @ts-expect-error Only used ad-hoc in dev
/*
* Compares the locally stored, filtered TCT root with the actual one on chain. They should match.
* This is expensive to do every block, so should only be done in development for debugging purposes.
* Note: In order for this to run, you must do this in the service worker console:
* globalThis.ASSERT_ROOT_VALID = true
*/
private async assertRootValid(blockHeight: bigint): Promise<void> {
const remoteRoot = await this.querier.cnidarium.fetchRemoteRoot(blockHeight);
const inMemoryRoot = this.viewServer.getSctRoot();

if (!remoteRoot.equals(inMemoryRoot)) {
throw new Error(
`Block height: ${blockHeight}. Wasm root does not match remote source of truth. Programmer error.`,
if (remoteRoot.equals(inMemoryRoot)) {
console.log(
`Block height: ${blockHeight} root matches remote ✅ \n`,
`Hash: ${uint8ArrayToHex(inMemoryRoot.inner)}`,
);
} else {
console.log(
`Block height: ${blockHeight} root does not match remote ❌ \n`,
`Local hash: ${uint8ArrayToHex(inMemoryRoot.inner)} \n`,
`Remote hash: ${uint8ArrayToHex(remoteRoot.inner)}`,
);
}
}
Expand Down
Loading