Treasury module E2E tests #439

dhirajs0 · 2025-10-09T16:12:27Z

End-to-end tests are added to cover the main flow of Treasury operations

Proposing a treasury spend
Approving and rejecting treasury proposals
Treasury payout execution
Handling proposal rejections and burns
Edge cases (e.g., insufficient funds, duplicate proposals)
Others

closes Issue #291

WIP

…AssetSpendVoided, sendPayoutTx, verifyEventPaid

…n assethub" This reverts commit f7bb7d5.

…etHub

… processed

…mits `InsufficientPermission` error

…paid

packages/shared/src/treasury.ts

github-actions

The changes introduce a comprehensive suite of E2E tests for the treasury module. While the test coverage is good, there are several issues in packages/shared/src/treasury.ts that need attention, including commented-out code, duplicated logic, potentially brittle helpers, unsafe type assertions, and comments indicating some tests are not functional. The new test file for Kusama Asset Hub is well-structured.

Suggestions that couldn't be attached to a specific line

packages/shared/src/treasury.ts:509, 522

The claimTreasurySpend test contains comments (// Not working after moving to asset hub and // assert(dispatchedEvent) not getting the dispatch event...) that indicate the test is broken or incomplete. Incomplete tests should be fixed, marked as .skip, or removed before merging to avoid having non-functional tests in the suite.

packages/shared/src/treasury.ts:370-425, 489-562, 592-666

The test functions voidApprovedTreasurySpendProposal, claimTreasurySpend, and checkStatusOfTreasurySpend contain significant duplicated code for creating and verifying a spend proposal. This setup logic should be extracted into a reusable helper function to reduce code duplication and improve maintainability.

github-actions · 2025-10-23T01:32:52Z

packages/shared/src/treasury.ts


 import { assert, expect } from 'vitest'

+//import { logAllEvents } from './helpers/helper_functions.js'


A commented-out import //import { logAllEvents } from './helpers/helper_functions.js' is present. Unused and commented-out code should be removed to improve code clarity.

github-actions · 2025-10-23T01:32:52Z

packages/shared/src/treasury.ts

+  const [event] = (await assetHubClient.api.query.system.events()).filter(
+    ({ event }: any) => event.section === 'treasury' && event.method === eventType,


The getSpendIndexFromEvent helper function filters for an event and directly accesses the first element of the result. This assumes only one event of the specified type will be present in the block's events. This could cause flaky tests if multiple such events are emitted. The function should be made more robust to handle cases where multiple events are found, or it should be documented why only one is expected. Additionally, the event is typed as any, which is not type-safe.

github-actions · 2025-10-23T01:32:52Z

packages/shared/src/treasury.ts

+  // const initialSpendCount = await getSpendCount(assetHubClient)
+  const initialSpendCount = (await assetHubClient.api.query.treasury.spendCount()).toNumber()


There is a commented-out line of code (// const initialSpendCount = ...) which should be removed. Immediately after, the logic from the getSpendCount helper function is duplicated. You should use the existing getSpendCount helper for consistency and to avoid code duplication.

github-actions · 2025-10-23T01:32:52Z

packages/shared/src/treasury.ts

+  const pendingOrFailedSpends = spends.filter((spend) => {
+    const spendData = spend[1]?.unwrap()
+    return (
+      (spendData?.status.isPending || spendData?.status.isFailed) && // not pending or failed


The comment // not pending or failed is inconsistent with the code (spendData?.status.isPending || spendData?.status.isFailed). The comment should be corrected to // is pending or failed to accurately reflect the logic.

github-actions · 2025-10-23T01:32:53Z

packages/shared/src/treasury.ts

+
+  // call payout tx for each pending or failed spend
+  for (const spend of pendingOrFailedSpends) {
+    const spendIndex = spend[0].toHuman?.() as number


The code spend[0].toHuman?.() as number is an unsafe way to get the spend index from a StorageKey. toHuman() can return complex objects, and the type assertion as number may fail at runtime. Please use the safer method spend[0].args[0].toNumber() to extract the index.

github-actions · 2025-10-23T01:32:53Z

packages/shared/src/treasury.ts

+  // verify the spends status is attempted
+  for (const spendIndex of spendIndices) {
+    const spend = await assetHubClient.api.query.treasury.spends(spendIndex)
+    expect(spend?.unwrap()?.status.isAttempted).toBe(true)


The expression spend?.unwrap()?.status.isAttempted can cause a runtime error if spend is None. unwrap() will be called on undefined and throw. You should first assert that the Option is Some before unwrapping it. For example: expect(spend.isSome).toBe(true); expect(spend.unwrap().status.isAttempted).toBe(true);.

dhirajs0 · 2025-10-28T11:44:33Z

All tests are passing for KAH locally.

dhirajs0 · 2025-10-29T11:45:00Z

All the tests are now working on the PAH as well locally tested.

rockbmb · 2025-10-29T15:59:46Z

packages/shared/src/treasury.ts

+  for (const spendIndex of spendIndices) {
+    const checkStatusEvents = await sendCheckStatusTx(assetHubClient, spendIndex)
+    await assetHubClient.dev.newBlock()
+    await verifyEventSpendProcessed(checkStatusEvents)


You might not want this here: from what I can tell, this verifyEventSpendProcessed helper matches against a snapshot; this is being done from within a loop, whose iteration count is unknown.
There could be 1 approved spend in the chain this is running on, or 1000.

For each iteration, a snapshot will be created, which might have already become obsolete by the time the next test run begins.

It is better to just inspect for the presence of the event without snapshotting it.

Thanks. I got it. I will update it.

I see you moved these tests to paritytech/ahm-dryrun#261, so this is not urgent.

However, for future reference, checking the same snapshot multiple times in one test is very often unintended.

Yes, since I have already added the test in the ahm-dryrun repo. I will be removing it from here, as it will be redundant here.

rockbmb mentioned this pull request Oct 11, 2025

Port bounties E2E tests to Kusama AH #441

Merged

dhirajs0 marked this pull request as ready for review October 21, 2025 11:58

dhirajs0 added 22 commits October 21, 2025 23:52

Basic treasury spend functionality test added with helper functions

c1ee3a2

test renamed

73915c1

test added: Void a previously approved proposal

6eb3c4c

spend and reejct origin defined

b1c4dcf

test added: claim a spend

aca4312

testAccounts.alice.address funded with intiail usdt balance on assethub

2c6ff8f

createSpendProposal() helper function added

3c2056e

helper function verifySystemEventAssetSpendApproved() added

c2cccc6

helper functions added : voidApprovedSpendProposal, verifySystemEvent…

7e31c3a

…AssetSpendVoided, sendPayoutTx, verifyEventPaid

added initial usdt funding to alice account on asset hub

2819a0d

Revert "testAccounts.alice.address funded with intiail usdt balance o…

96b1227

…n assethub" This reverts commit f7bb7d5.

linter fix

4bf0c7f

import fix

857da95

snapshot updated

4abdba1

helpers added getAssetHubBalanceAmount and setInitialUSDTBalanceOnAss…

cacb40d

…etHub

test added: Check status of a spend and remove it from the storage if…

4ea2473

… processed

test added: Proposing a expired spend emits SpendExpired error

4d24339

test added: Smalltipper trying to spend more than the origin allows e…

b7d6ab3

…mits `InsufficientPermission` error

updated snapshot

4acb1df

treasury module added in assetHubKusama

c016eaf

test added: Check treasury payouts which are already approved can be …

716d62b

…paid

all tests ported to assetHub from relay chain

9e728fb

dhirajs0 force-pushed the treasury-e2e-test branch from a91c6fd to 9e728fb Compare October 22, 2025 14:32

dhirajs0 commented Oct 22, 2025

View reviewed changes

packages/shared/src/treasury.ts Show resolved Hide resolved

dhirajs0 commented Oct 22, 2025

View reviewed changes

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

github-actions bot reviewed Oct 23, 2025

View reviewed changes

dhirajs0 added 2 commits October 26, 2025 16:27

blockProvide added for scheduleInlineCallWithOrigin

ba033d5

asset kind of spend changed to native

09d5d2e

dhirajs0 added 12 commits October 28, 2025 14:16

sendCheckStatusTx ported to Asset hub

5fe4aed

check_status called for treasury payouts

4d5cbeb

updated

36e1f5c

ASSET_KIND and BENEFICIARY_LOCATION modified for assethub

ba6c795

largeSpendAmount with smalltipper

1cad7fd

block provided added in voidSpend

cb4e246

usdt balance comparison replaced with native balance

b12423c

unused usdt funding functions removed

819a80c

initial usdt funding removed

f554efa

old test treasurySpendForeignAssetTest for relay chain removed

dd852a0

lint fixed

8133bd0

snapshot for treasury assethub pallet

9ca3f9a

treasury deleted from polkadot relay

40b55a1

dhirajs0 requested a review from rockbmb October 28, 2025 11:46

dhirajs0 added 4 commits October 28, 2025 19:52

assetHubPolkadot e2e module added

d14fe1a

fixed tests failing on the PAH due to multiple dispatched events

ccfd8db

refac

b2c2bc6

snaps assetHubPolkadot treasury updated

1039966

rockbmb reviewed Oct 29, 2025

View reviewed changes


		import { assert, expect } from 'vitest'

		//import { logAllEvents } from './helpers/helper_functions.js'

		const [event] = (await assetHubClient.api.query.system.events()).filter(
		({ event }: any) => event.section === 'treasury' && event.method === eventType,

		// const initialSpendCount = await getSpendCount(assetHubClient)
		const initialSpendCount = (await assetHubClient.api.query.treasury.spendCount()).toNumber()

Treasury module E2E tests #439

Are you sure you want to change the base?

Treasury module E2E tests #439

Uh oh!

Conversation

dhirajs0 commented Oct 9, 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

Suggestions that couldn't be attached to a specific line

Uh oh!

github-actions bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

github-actions bot Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

dhirajs0 commented Oct 28, 2025

Uh oh!

dhirajs0 commented Oct 29, 2025

Uh oh!

rockbmb Oct 29, 2025

Choose a reason for hiding this comment

Uh oh!

dhirajs0 Oct 30, 2025

Choose a reason for hiding this comment

Uh oh!

rockbmb Nov 1, 2025

Choose a reason for hiding this comment

Uh oh!

dhirajs0 Nov 2, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dhirajs0 commented Oct 9, 2025 •

edited

Loading