Add tests for PreImage (continued) #471

andreitrand · 2025-11-04T13:33:58Z

Add additional tests which verify PreImage behavior:

1. A manager account requests a preimage hash once then unrequests it multiple times (added additional checks)
1. Check the noting and unnoting of empty images
1. Check the noting and unnoting of oversized images
1. Check the repeated noting and unnoting of the same preimage
1. Check preimage behavior when requesting, noting, unrequesting and unnoting occur in different orders
1. Check the "ensure_updated" extrinsic applies fees depending on the update ratio

Several utility functions have been moved to the helpers file as they are shared with other test files.

This is a continuation of the PR: #470.
Also address outstanding comments from the original PR.

Closes #453.

github-actions

The code changes introduce several new tests for preimage functionality. However, two of the new tests (preimageOversizedTest and preimageEnsureUpdatedTest) are incomplete as they contain commented-out assertions and TODOs. Additionally, some new test functions have very similar names which could lead to confusion. The refactoring in other files looks good.

Suggestions that couldn't be attached to a specific line

packages/shared/src/preimage.ts:262, 431

The function preimageRequestAndNoteTest was renamed to preimageNoteAndRequestTest (line 262), and a new test also named preimageRequestAndNoteTest was added (line 431). These names are very similar and can be easily confused. For better clarity, consider renaming them to more explicitly describe the order of operations, such as preimageNoteThenRequestTest and preimageRequestThenNoteTest.

packages/shared/src/preimage.ts

andreitrand · 2025-11-12T14:20:19Z

The code changes introduce several new tests for preimage functionality. However, two of the new tests (preimageOversizedTest and preimageEnsureUpdatedTest) are incomplete as they contain commented-out assertions and TODOs. Additionally, some new test functions have very similar names which could lead to confusion. The refactoring in other files looks good.

Suggestions that couldn't be attached to a specific line

packages/shared/src/preimage.ts:262, 431

The function preimageRequestAndNoteTest was renamed to preimageNoteAndRequestTest (line 262), and a new test also named preimageRequestAndNoteTest was added (line 431). These names are very similar and can be easily confused. For better clarity, consider renaming them to more explicitly describe the order of operations, such as preimageNoteThenRequestTest and preimageRequestThenNoteTest.

Fixed.

rockbmb · 2025-11-13T15:04:42Z

Will close #453 .

packages/polkadot/src/__snapshots__/polkadot.preimage.e2e.test.ts.snap

packages/shared/src/preimage.ts

github-actions

Found a logic issue in the new helper function scheduleCallListWithOrigin which incorrectly schedules multiple calls for the same block, causing only the last one to be executed. This also affects the preimageSingleRequestMultipleUnrequestTest which currently passes but verifies incorrect behavior. Additionally, there are commented-out assertions in the new tests that should be addressed.

github-actions · 2025-11-19T21:13:09Z

packages/shared/src/helpers/index.ts

+  const agenda = calls.map(({ call, origin }) => [
+    [scheduledBlock],
+    [
+      {
+        call,
+        origin: origin,
+      },
+    ],
+  ])


The agenda construction creates multiple entries with the same key ([scheduledBlock]) when calls has more than one element. setStorage will likely overwrite previous entries with the same key, resulting in only the last call being scheduled. You should aggregate all calls into a single entry for the scheduledBlock.

Suggested fix:

const agenda = [ [ [scheduledBlock], calls.map(({ call, origin }) => ({ call, origin, })), ], ]

github-actions · 2025-11-19T21:13:09Z

packages/shared/src/preimage.ts

+
+  // A "Dispatched" event does appear from the scheduler.
+  events = await getEventsWithType(client, 'scheduler')
+  expect(events.length).toBe(1)


The test expects exactly 1 Dispatched event, which confirms that only one of the three unrequest calls is being executed due to the bug in scheduleCallListWithOrigin. Once the helper is fixed to schedule all 3 calls, this expectation should be updated to expect 3 Dispatched events (one for each call, even if they are no-ops).

github-actions · 2025-11-19T21:13:10Z

packages/shared/src/preimage.ts

+  // expect((await getEventsWithType(client, 'preimage')).length).toBe(0)
+  // expect((await getEventsWithType(client, 'system')).length).toBeGreaterThan(0)
+  // expectFailedExtrinsicWithType(client, client.api.errors.preimage.PreimageTooLarge)


These assertions are commented out. If the test is intended to verify that the extrinsic fails and no preimage events are emitted, these should be uncommented. If there is a reason they are disabled (e.g., pending functionality), the TODO should be more descriptive or the code removed.

Reorganized the TODO messages.

github-actions · 2025-11-19T21:13:10Z

packages/shared/src/preimage.ts

+  // 6. If the ratio of new to total preimages is less than 90%, check that fees have been paid.
+  if (expectFees) {
+    // TODO: The "ensure_updated" fee should be strictly positive in this case!
+    // expect(ensureUpdatedFee).toBeGreaterThan(0)


The assertion expect(ensureUpdatedFee).toBeGreaterThan(0) is commented out with a TODO. If the fee calculation is expected to be positive, this should be fixed or explained.

Added details for TODO.

rockbmb · 2025-11-20T03:46:04Z

Found a logic issue in the new helper function scheduleCallListWithOrigin which incorrectly schedules multiple calls for the same block, causing only the last one to be executed. This also affects the preimageSingleRequestMultipleUnrequestTest which currently passes but verifies incorrect behavior. Additionally, there are commented-out assertions in the new tests that should be addressed.

It's right! 😮

Code as-is:

With the fix:

Tested with yarn test preimage -t "preimage single request and multiple unrequest test", with a pause right before scheduleInlineCallListWithSameOrigin.

andreitrand · 2025-11-20T14:55:46Z

Found a logic issue in the new helper function scheduleCallListWithOrigin which incorrectly schedules multiple calls for the same block, causing only the last one to be executed. This also affects the preimageSingleRequestMultipleUnrequestTest which currently passes but verifies incorrect behavior. Additionally, there are commented-out assertions in the new tests that should be addressed.

It's right! 😮

Code as-is:

With the fix:

Tested with yarn test preimage -t "preimage single request and multiple unrequest test", with a pause right before scheduleInlineCallListWithSameOrigin.

Fixed with the suggested solution.

Add additional tests which verify PreImage behavior: - 1) A manager account requests a preimage hash once then unrequests it multiple times (added additional checks) - 2) Check the noting and unnoting of empty images - 3) Check the noting and unnoting of oversized images - 4) Check the repeated noting and unnoting of the same preimage - 5) Check preimage behavior when requesting, noting, unrequesting and unnoting occur in different orders - 6) Check the "ensure_updated" extrinsic applies fees depending on the update ratio Several utility functions have been moved to the helpers file as they are shared with other test files. This is a continuation of the PR: open-web3-stack#470. Also address outstanding comments from the original PR.

rockbmb · 2025-11-20T16:38:48Z

Merged, thanks @andreitrand !

andreitrand marked this pull request as draft November 4, 2025 13:34

andreitrand mentioned this pull request Nov 4, 2025

Add tests for PreImage #470

Merged

andreitrand force-pushed the andreitrand-pet-tests-preimage-additional branch 4 times, most recently from 9246af8 to f9f62b5 Compare November 11, 2025 12:40

andreitrand marked this pull request as ready for review November 11, 2025 12:41

andreitrand force-pushed the andreitrand-pet-tests-preimage-additional branch 2 times, most recently from f0478e4 to 68b834c Compare November 11, 2025 16:33

github-actions bot reviewed Nov 11, 2025

View reviewed changes

packages/shared/src/preimage.ts Outdated Show resolved Hide resolved

packages/shared/src/preimage.ts Outdated Show resolved Hide resolved

andreitrand force-pushed the andreitrand-pet-tests-preimage-additional branch from 68b834c to a023900 Compare November 12, 2025 14:19

rockbmb added the e2e tests Related to end-to-end tests label Nov 13, 2025

rockbmb reviewed Nov 14, 2025

View reviewed changes

packages/polkadot/src/__snapshots__/polkadot.preimage.e2e.test.ts.snap Outdated Show resolved Hide resolved

packages/shared/src/preimage.ts Outdated Show resolved Hide resolved

rockbmb approved these changes Nov 16, 2025

View reviewed changes

andreitrand force-pushed the andreitrand-pet-tests-preimage-additional branch 2 times, most recently from 1348d53 to 4b7d54e Compare November 19, 2025 18:43

github-actions bot reviewed Nov 19, 2025

View reviewed changes

andreitrand force-pushed the andreitrand-pet-tests-preimage-additional branch from 22e6522 to a19cb6b Compare November 20, 2025 15:07

rockbmb merged commit 20f9bf5 into open-web3-stack:master Nov 20, 2025
216 of 222 checks passed

This was referenced Nov 20, 2025

Add minor missing points to Preimage test #481

Closed

Addenda to preimage tests #482

Merged

Add tests for PreImage (continued) #471

Add tests for PreImage (continued) #471

Uh oh!

Conversation

andreitrand commented Nov 4, 2025 • edited by rockbmb Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Suggestions that couldn't be attached to a specific line

Uh oh!

Uh oh!

Uh oh!

andreitrand commented Nov 12, 2025

Suggestions that couldn't be attached to a specific line

Uh oh!

rockbmb commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

andreitrand Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

andreitrand Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

andreitrand Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Nov 19, 2025

Choose a reason for hiding this comment

Uh oh!

andreitrand Nov 20, 2025

Choose a reason for hiding this comment

Uh oh!

rockbmb commented Nov 20, 2025

Uh oh!

andreitrand commented Nov 20, 2025

Uh oh!

Uh oh!

rockbmb commented Nov 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

andreitrand commented Nov 4, 2025 •

edited by rockbmb

Loading

rockbmb commented Nov 13, 2025 •

edited

Loading