refactor(test-loop): migrate transaction helpers to tx_* node API by pugachAG · Pull Request #15305 · near/nearcore

pugachAG · 2026-03-05T09:01:35Z

Remove do_create_account, do_delete_account, do_deploy_contract, do_call_contract from transactions.rs — replaced by tx_* API on TestLoopNode combined with NodeRunner::run_tx()
Remove test_create_delete_account (covered by test_create_and_delete_account in examples/basic.rs)
Migrate contract_distribution_simple tests to use rpc_node()/rpc_runner() by adding a dedicated RPC node to the test setup
Move create_account helper to resharding_v3.rs (its only caller)
Replace delete_account usage in resharding.rs with TestLoopNode::tx_delete_account
Remove deploy_contract, call_contract, deploy_global_contract, use_global_contract, prepare_transfer_tx, submit_tx from transactions.rs — all replaced by corresponding tx_* methods on TestLoopNode
Add tx_deploy_global_contract and tx_use_global_contract to TestLoopNode API
Move store_and_submit_tx, submit_tx, check_txs_remove_successful to resharding.rs (their only consumer) - will be refactored separately as part of resharding_v3 test overhaul
Migrate congestion_control, contract_distribution_cross_shard, global_contracts_distribution tests to use the node API directly

This is follow-up to #15296.

- Remove do_create_account, do_delete_account, do_deploy_contract, do_call_contract from transactions.rs — replaced by the simplified tx_* API on TestLoopNode combined with NodeRunner::run_tx() - Remove test_create_delete_account (covered by test_create_and_delete_account in examples/basic.rs) - Migrate contract_distribution_simple tests to use rpc_node()/rpc_runner() by adding a dedicated RPC node to the test setup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…e API Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

… resharding.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ding test node.tx_delete_account() uses node.head().last_block_hash, which is the head of a single node. With shard shuffling enabled, nodes can be at different heights and the chunk producer that processes the tx might not know about that block hash yet, causing the tx to be silently dropped. Use get_shared_block_hash() instead, which picks the block at the minimum head height across all nodes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

codecov · 2026-03-06T08:28:47Z

Codecov Report

❌ Patch coverage is 93.75000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.25%. Comparing base (e6d5328) to head (7dfa78d).
⚠️ Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
test-loop-tests/src/utils/resharding.rs	90.76%	4 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #15305      +/-   ##
==========================================
- Coverage   69.30%   69.25%   -0.05%     
==========================================
  Files         931      931              
  Lines      207312   207148     -164     
  Branches   207312   207148     -164     
==========================================
- Hits       143668   143467     -201     
- Misses      57725    57772      +47     
+ Partials     5919     5909      -10

Flag	Coverage Δ
pytests-nightly	`1.27% <ø> (-0.01%)`	⬇️
unittests	`68.61% <93.75%> (-0.05%)`	⬇️
unittests-nightly	`68.82% <93.75%> (-0.05%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ntract with tx_* node API Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…ode API Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot

Pull request overview

Refactors test-loop-tests transaction submission in tests/utilities to rely on TestLoopNode::tx_* builders plus NodeRunner::run_tx()/execute_tx(), removing duplicated transaction helper functions and updating affected tests accordingly.

Changes:

Removes many ad-hoc tx helper functions from utils/transactions.rs, migrating call sites to TestLoopNode/NodeRunner.
Adds tx_deploy_global_contract and tx_use_global_contract to the TestLoopNode API and updates global-contract-related tests to use them.
Refactors resharding utilities/tests to use node APIs directly and relocates the few remaining resharding-specific tx helpers into utils/resharding.rs.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test-loop-tests/src/utils/transactions.rs	Deletes legacy tx helper functions; keeps shared query/tx-status helpers used across tests.
test-loop-tests/src/utils/resharding.rs	Migrates transfer submission to `TestLoopNode`; inlines resharding-only tx tracking helpers.
test-loop-tests/src/utils/node.rs	Extends node tx builder API with global-contract deploy/use helpers.
test-loop-tests/src/tests/resharding_v3.rs	Switches setup txs to node API and adds a local `create_account` helper.
test-loop-tests/src/tests/global_contracts_distribution.rs	Migrates global contract distribution tests to node tx API + runners.
test-loop-tests/src/tests/create_delete_account.rs	Removes the old create/delete flow test, leaving the instant DeleteAccount receipt test.
test-loop-tests/src/tests/contract_distribution_simple.rs	Updates contract distribution tests to use a dedicated RPC node and node tx API.
test-loop-tests/src/tests/contract_distribution_cross_shard.rs	Updates cross-shard contract distribution test to node tx API.
test-loop-tests/src/tests/congestion_control.rs	Updates congestion control tests to use node tx API and node runners.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-06T10:12:07Z

                let anchor_hash = get_anchor_hash(&clients);
-                let nonce = get_next_nonce(&test_loop_data, &node_datas, &sender);
+                let node = TestLoopNode {
+                    data: test_loop_data,
+                    node_data: get_node_data(node_datas, &client_account_id),
+                };
+                let nonce = node.get_next_nonce(&sender);
                let amount = Balance::from_near(1).checked_mul(rng.gen_range(1..=10)).unwrap();


tx_send_money() uses self.head().last_block_hash as the anchor hash. In this module you already note (later) that using a single node’s head can break when shard shuffling causes nodes to be at different heights (the chunk producer may not know that block yet). Consider building transfer txs using get_shared_block_hash(node_datas, test_loop_data) (or a shared-head helper) instead of the local head, at least for resharding/shard-shuffling loop actions.

Copilot · 2026-03-06T10:12:07Z

+        SignedTransaction::deploy_global_contract(
+            self.get_next_nonce(deployer_id),
+            deployer_id.clone(),
+            code,
+            &create_user_test_signer(deployer_id),
+            self.head().last_block_hash,
+            deploy_mode,
+        )


tx_deploy_global_contract anchors the tx to self.head().last_block_hash. Previously the test-loop transaction helpers used a shared/minimum head hash across nodes to ensure every node (including the chunk producer) recognizes the referenced block, which matters when nodes are at different heights (e.g. with shard shuffling). Consider adding an API that accepts an explicit block_hash or otherwise uses a shared block hash for building these txs.

Copilot · 2026-03-06T10:12:07Z

+        SignedTransaction::use_global_contract(
+            self.get_next_nonce(user_id),
+            user_id,
+            &create_user_test_signer(user_id),
+            self.head().last_block_hash,
+            identifier,
+        )


Same anchoring concern as tx_deploy_global_contract: tx_use_global_contract uses self.head().last_block_hash. In shard-shuffling / multi-node setups this can produce txs referencing a block unknown to the node that ends up processing the tx. Consider allowing callers to pass a shared block hash (or providing a *_with_block_hash variant).

Copilot · 2026-03-06T10:12:08Z

+    let tx = SignedTransaction::create_account(
+        nonce,
+        originator.clone(),
+        new_account_id.clone(),
+        amount,
+        new_signer.public_key(),
+        &signer,
+        node.head().last_block_hash,
+    );


This helper now anchors the create-account tx to node.head().last_block_hash. The old helper in utils/transactions.rs used get_shared_block_hash(...) to avoid referencing a block that some nodes (or the eventual chunk producer) may not have yet when shard shuffling causes height skew. Consider switching back to get_shared_block_hash(&env.node_datas, &env.test_loop.data) (or passing an explicit anchor hash) to preserve the previous, more robust behavior.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

darioush

nice cleanup, leaving some suggestions

darioush · 2026-03-06T13:05:32Z

 use crate::utils::get_node_head_height;
-use crate::utils::transactions::{call_contract, check_txs, deploy_contract, make_accounts};
+use crate::utils::transactions::{check_txs, make_accounts};
+use near_primitives::gas::Gas;


nit: import group ordering, seems should be merged with above group which has near_primitives

Separately, it would be great to have a linter for this, to avoid spending time on formatting in code review. Let me create an issue: #15329

darioush · 2026-03-06T13:14:57Z

+    let tx = env.rpc_node().tx_delete_account(&contract_id, &beneficiary);
+    env.rpc_runner().run_tx(tx, Duration::seconds(5));


Looking at this and other tests, I wonder if the distinction of runner and node is useful from the client perspective?

node doesn't require mutable reference to TestLoopData, so there can be multiple instances and it can be derived from &TestLoopEnv.
do you think it would be worthwhile to merge everything into single node api?

also see our previous related discussion here

another important consideration: NodeRunner::run_until condition operates on node instance, so it would be hard to merge those together

It is already a great improvement, we can keep it for now. The mut reference problem seems worse.

darioush · 2026-03-06T13:21:45Z

+                    near_primitives::test_utils::create_user_test_signer(&temporary_account_id);
+                let nonce = node.get_next_nonce(&temporary_account_id);
+                let block_hash = get_shared_block_hash(node_datas, test_loop_data);
+                let tx = near_primitives::transaction::SignedTransaction::delete_account(


nit: use style import

sorry for that, claude refuses to comply with CLAUDE.md instructions 😞

yeah, I noticed it too. probably we need some script.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

pugachAG force-pushed the refactor-test-loop-replace-do-methods branch 2 times, most recently from e52f932 to 2c8b3c8 Compare March 5, 2026 09:57

pugachAG and others added 4 commits March 6, 2026 09:15

refactor(test-loop): move create_account to resharding_v3.rs, use nod…

1dd63f3

…e API Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

refactor(test-loop): replace delete_account with tx_delete_account in…

3c6a29f

… resharding.rs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

pugachAG force-pushed the refactor-test-loop-replace-do-methods branch from c975956 to 7a6eaf3 Compare March 6, 2026 08:19

pugachAG and others added 6 commits March 6, 2026 10:09

remove deploy_contract

440d0f4

refactor(test-loop): replace call_contract with tx_call node API

4a01274

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

refactor(test-loop): replace deploy_global_contract and use_global_co…

1cfa9e5

…ntract with tx_* node API Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

refactor(test-loop): replace prepare_transfer_tx with tx_send_money n…

e08ca50

…ode API Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

refactor(test-loop): move store_and_submit_tx to resharding.rs

2a96dd4

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

refactor(test-loop): move check_txs_remove_successful to resharding.rs

9a95aa1

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

pugachAG changed the title ~~refactor(test-loop): replace do_* methods with tx_* node API~~ refactor(test-loop): migrate transaction helpers to tx_* node API Mar 6, 2026

pugachAG requested review from darioush and shreyan-gupta March 6, 2026 10:02

pugachAG marked this pull request as ready for review March 6, 2026 10:03

pugachAG requested a review from a team as a code owner March 6, 2026 10:03

Copilot AI review requested due to automatic review settings March 6, 2026 10:03

Copilot started reviewing on behalf of pugachAG March 6, 2026 10:03 View session

pugachAG force-pushed the refactor-test-loop-replace-do-methods branch from 06fe160 to 9dd2d95 Compare March 6, 2026 10:09

Copilot AI reviewed Mar 6, 2026

View reviewed changes

pugachAG force-pushed the refactor-test-loop-replace-do-methods branch from 9dd2d95 to d68752b Compare March 6, 2026 10:22

refactor(test-loop): remove submit_tx, replace with node API

a5cea57

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

pugachAG force-pushed the refactor-test-loop-replace-do-methods branch from d68752b to a5cea57 Compare March 6, 2026 10:24

darioush approved these changes Mar 6, 2026

View reviewed changes

darioush reviewed Mar 6, 2026

View reviewed changes

Comment thread test-loop-tests/src/tests/congestion_control.rs

pugachAG and others added 2 commits March 6, 2026 16:47

refactor(test-loop): return tx hash from TestLoopNode::submit_tx

8960d74

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

use

7dfa78d

pugachAG added this pull request to the merge queue Mar 6, 2026

Merged via the queue into master with commit 0bc095b Mar 6, 2026
27 checks passed

pugachAG deleted the refactor-test-loop-replace-do-methods branch March 6, 2026 16:28

		let tx = env.rpc_node().tx_delete_account(&contract_id, &beneficiary);
		env.rpc_runner().run_tx(tx, Duration::seconds(5));

Conversation

pugachAG commented Mar 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

darioush left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pugachAG Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pugachAG commented Mar 5, 2026 •

edited

Loading

codecov Bot commented Mar 6, 2026 •

edited

Loading

pugachAG Mar 6, 2026 •

edited

Loading