feat: changing node account ids #3507
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
src/client/WebClient.js
Outdated
| for (const nodeAddress of addressBook.nodeAddresses) { | ||
| for (const endpoint of nodeAddress.addresses) { | ||
| if (nodeAddress.accountId != null) { | ||
| // Exclude nodes with account ID 0.0.0 (removed node account IDs) |
There was a problem hiding this comment.
This logic was removed from the HIP, it's not possible to set node account id to 0 => remove this change
src/client/Network.js
Outdated
| if ( | ||
| endpoint.port === port && | ||
| nodeAddress.accountId != null && | ||
| nodeAddress.accountId.toString() !== "0.0.0" |
There was a problem hiding this comment.
This logic was removed from the HIP, it's not possible to set node account id to 0 => remove this change
| expect(receipt.status).to.equal(Status.Success); | ||
| }); | ||
|
|
||
| it("should successfully remove node account ID (set to 0.0.0) with admin key signature", async function () { |
There was a problem hiding this comment.
Tests are reworked please check again
| }); | ||
|
|
||
| describe("Node Account ID Update Operations", function () { | ||
| it("should successfully update node account ID with both admin and account key signatures", async function () { |
There was a problem hiding this comment.
this test should bring back the original node account id at the end of the test
There was a problem hiding this comment.
Tests are reworked please check
| if (error instanceof PrecheckStatusError) { | ||
| expect(error.status).to.equal(Status.InvalidAccountId); | ||
| } else if (error instanceof StatusError) { | ||
| expect(error.status).to.equal(Status.InvalidAccountId); |
| let originalNodeAccountId; | ||
|
|
||
| beforeAll(async function () { | ||
| env = await IntegrationTestEnv.new(); |
There was a problem hiding this comment.
this will not work in CI, because in CI we don't use account 0.0.2. You should hard-code the client's operator (0.0.2) and it's 2 nodes 0.0.3 and 0.0.4 here
|
|
||
| describe("SDK Retry and Network Update Behavior", function () { | ||
| it("should retry with another node when encountering INVALID_NODE_ACCOUNT_ID", async function () { | ||
| const transaction = new TransferTransaction() |
There was a problem hiding this comment.
nowhere in this test i see the node account being updated, if it's using some of the above tests for doing the update -> it's a bad test. It should be self contained. See go's test https://github.com/hiero-ledger/hiero-sdk-go/blob/4c901a9700c8c9aefde6ef779a0e109993a5976a/sdk/node_update_transaction_e2e_test.go#L246
venilinvasilev
force-pushed
the
feat/changing-node-account-ids
branch
from
55e9bfb to
62cb6b7
Compare
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
venilinvasilev
force-pushed
the
feat/changing-node-account-ids
branch
6 times, most recently
from
be6f7db to
2cf661a
Compare
venilinvasilev
changed the title
Signed-off-by: venilinvasilev <[email protected]>
Signed-off-by: venilinvasilev <[email protected]>
Signed-off-by: venilinvasilev <[email protected]>
Signed-off-by: venilinvasilev <[email protected]>
Signed-off-by: venilinvasilev <[email protected]>
Signed-off-by: venilinvasilev <[email protected]>
Signed-off-by: venilinvasilev <[email protected]>
Signed-off-by: venilinvasilev <[email protected]>
Signed-off-by: venilinvasilev <[email protected]>
venilinvasilev
force-pushed
the
feat/changing-node-account-ids
branch
from
1e6bf46 to
5e5fa1b
Compare
Summary
This PR implements the test plan for validating updates to the
account_idfield inNodeUpdateTransactionBodyas part of the DynamicAddress Book (DAB) enhancement. It also covers new SDK behaviors that
improve reliability when node account IDs become outdated or invalid.
Scope
Test coverage for
account_idupdatesaccount_id.SDK node-selection and retry behavior
Signing with all nodes
Ensures the SDK signs transactions with all nodes to reduce the
chance of failures caused by outdated node account IDs.
Client network refresh via mirror node
When an invalid node account error occurs, the SDK retrieves updated
network information from a mirror node and retries the transaction.
Fallback behavior without a mirror network
If no mirror node is configured, the SDK retries execution using the
next node ID in the transaction's node list until all options are
exhausted.
Alignment
The implementation aligns with the design described in:
"Changing of Node Account IDs for Nodes" (sdk-collaboration-hub#53)
Rationale
These tests ensure the robustness of Dynamic Address Book functionality,
reliable handling of account ID changes, and improved resilience in
transaction execution workflows.
Related issue(s):
#3370
Fixes #3370