Skip to content

Conversation

@rockbmb
Copy link
Collaborator

@rockbmb rockbmb commented Nov 21, 2025

Follow-up to #471 . Closes #481 .

@rockbmb rockbmb added this to the General CUI/CUJ Coverage milestone Nov 21, 2025
@rockbmb rockbmb self-assigned this Nov 21, 2025
@rockbmb rockbmb added enhancement New feature or request e2e tests Related to end-to-end tests labels Nov 21, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Found 3 issues in packages/shared/src/preimage.ts: one critical logic error regarding nonce usage, one performance/best-practice issue with array creation, and one documentation inconsistency.

Comment on lines +712 to +713
const oversizedBytesArray = Array(maxPreimageSize + 1).fill(1)
const oversizedBytes = client.api.createType('Bytes', oversizedBytesArray)
Copy link
Contributor

Choose a reason for hiding this comment

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

Using Array(maxPreimageSize + 1) creates a sparse/dense array of numbers which is memory inefficient for binary data. Use new Uint8Array(maxPreimageSize + 1).fill(1) instead, which createType supports directly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Unit8Array was used before, and due to a SCALE + PJS interaction that I didn't delve too deeply into, the maximum allowed size of the preimage ended up being 64 bytes.

@andreitrand not a cabal answer, but here's the fix!

@rockbmb rockbmb requested a review from xlc November 21, 2025 22:54
@rockbmb rockbmb enabled auto-merge (squash) November 22, 2025 12:21
@rockbmb rockbmb merged commit 0aefa5d into master Nov 25, 2025
112 of 114 checks passed
@rockbmb rockbmb deleted the preimage-addenda branch November 25, 2025 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e tests Related to end-to-end tests enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add minor missing points to Preimage test

3 participants