-
Notifications
You must be signed in to change notification settings - Fork 623
fix(pxe): cap fresh secret pending tag indexes to the probed window #24667
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
Changes from all commits
7b9c3bc
bf1c81c
5e126e9
dfd735c
971e31b
d379d50
325c8a9
9521cce
b41b678
76e8df0
8a48abd
33cc14a
b6bce2d
aa12efd
0402750
bbc6d21
c5526c1
bc7db58
5aeb4af
1ff0a62
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,7 @@ import { AppTaggingSecret, SiloedTag, type TaggingIndexRange } from '@aztec/stdl | |
| import { TxEffect, TxHash } from '@aztec/stdlib/tx'; | ||
|
|
||
| import type { StagedStore } from '../../job_coordinator/job_coordinator.js'; | ||
| import { UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN } from '../../tagging/constants.js'; | ||
| import { UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN, unfinalizedTaggingIndexesWindowEnd } from '../../tagging/constants.js'; | ||
|
|
||
| /** Internal representation of a pending index range entry. */ | ||
| type PendingIndexesEntry = { lowestIndex: number; highestIndex: number; txHash: string }; | ||
|
|
@@ -195,13 +195,9 @@ export class SenderTaggingStore implements StagedStore { | |
|
|
||
| // Process in memory and validate | ||
| for (const { range, secretStr, pendingData, finalizedIndex } of rangeData) { | ||
| // Check that the highest index is not further than window length from the highest finalized index. | ||
| if (range.highestIndex > (finalizedIndex ?? 0) + UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Highest index can be |
||
| throw new Error( | ||
| `Highest used index ${range.highestIndex} is further than window length from the highest finalized index ${finalizedIndex ?? 0}. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my understanding, if there is no finalized index, the highest finalized index is not 0, 0 is still available to be used as a finalized index. |
||
| Tagging window length ${UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN} is configured too low. Contact the Aztec team | ||
| to increase it!`, | ||
| ); | ||
| const windowEnd = unfinalizedTaggingIndexesWindowEnd(finalizedIndex); | ||
| if (range.highestIndex >= windowEnd) { | ||
| throw windowExceededError(range.highestIndex, windowEnd, finalizedIndex); | ||
| } | ||
|
|
||
| // Throw if the lowest index is lower than or equal to the last finalized index | ||
|
|
@@ -521,3 +517,18 @@ export class SenderTaggingStore implements StagedStore { | |
| } | ||
| } | ||
| } | ||
|
|
||
| /** Builds the error thrown when a pending tag index is at or past the unfinalized tagging window end. */ | ||
| export function windowExceededError( | ||
| highestIndex: number, | ||
| windowEnd: number, | ||
| finalizedIndex: number | undefined, | ||
| ): Error { | ||
| const finalizedDescription = | ||
| finalizedIndex === undefined ? 'no index finalized yet' : `highest finalized index ${finalizedIndex}`; | ||
| return new Error( | ||
| `Highest used index ${highestIndex} is at or past the window end ${windowEnd} (${finalizedDescription}). ` + | ||
| `Tagging window length ${UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN} is configured too low. ` + | ||
| `Contact the Aztec team to increase it!`, | ||
| ); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,6 +19,18 @@ import { MAX_PRIVATE_LOGS_PER_TX } from '@aztec/constants'; | |
| // MAX_PRIVATE_LOGS_PER_TX. No fixed window value closes that gap. | ||
| export const UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN = MAX_PRIVATE_LOGS_PER_TX + 20; | ||
|
|
||
| /** | ||
| * Exclusive upper bound of the tag indexes that can exist for a secret whose highest finalized index is | ||
| * `finalizedIndex` (undefined when nothing is finalized yet). The sender store refuses pending indexes at or past it, | ||
| * and both sender and recipient sync scan exactly up to it. Every absolute window bound must come from this helper: | ||
| * a site with a wider or narrower bound lets a tx land at an index the syncs never scan, so two stores sharing the | ||
| * secret could later pick a colliding index. | ||
| */ | ||
| export function unfinalizedTaggingIndexesWindowEnd(finalizedIndex: number | undefined): number { | ||
| const windowStart = finalizedIndex === undefined ? 0 : finalizedIndex + 1; | ||
| return windowStart + UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN; | ||
| } | ||
|
Comment on lines
+29
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we make this return
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't want to return |
||
|
|
||
| // The number of tags probed per constrained secret in the first round. | ||
| // | ||
| // The probe doubles each round (2, 4, 8, ..., capped at UNFINALIZED_TAGGING_INDEXES_WINDOW_LEN) while every probed | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The crux of the issue. We allowed storing WINDOW + 1 indices rather than WINDOW indices for a fresh secret.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦