Skip to content

Addenda to preimage tests#482

Merged
rockbmb merged 7 commits into
masterfrom
preimage-addenda
Nov 25, 2025
Merged

Addenda to preimage tests#482
rockbmb merged 7 commits into
masterfrom
preimage-addenda

Conversation

@rockbmb

@rockbmb rockbmb commented Nov 21, 2025

Copy link
Copy Markdown
Collaborator

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

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
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!

Comment thread packages/shared/src/preimage.ts Outdated
Comment thread packages/shared/src/preimage.ts Outdated
@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

2 participants