Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TransactionReceipt nodeId property doesn't appear to be set properly for DAB transactions #16595

Open
jbair06 opened this issue Nov 13, 2024 · 3 comments · May be fixed by #16622
Open

TransactionReceipt nodeId property doesn't appear to be set properly for DAB transactions #16595

jbair06 opened this issue Nov 13, 2024 · 3 comments · May be fixed by #16622
Assignees

Comments

@jbair06
Copy link
Member

jbair06 commented Nov 13, 2024

Problem

I ran a NodeDelete transaction. I submitted the transaction to node 6 (with account ID 0.0.9). I deleted node 2. The receipt had the nodeId set to 0. After testing a few different times, submitted to different nodes and deleting different nodes, it appears that the nodeId never gets set for the NodeDelete.

Solution

  • NodeDelete and NodeUpdate both need to set the nodeId of the affected node.

Alternatives

No response

@jbair06
Copy link
Member Author

jbair06 commented Nov 14, 2024

After discussion with @iwsimon and @netopyr, the NodeDelete and NodeUpdate receipts do not need to include the nodeId.

The nodeId for these two transactions is similar to the accountId to the account update and delete transactions. Meaning that the account id is not included in the receipt, as it was part of the request. So, too, the nodeId is not needed in the receipt as it is part of the request.

Therefore, the documentation should be updated to reflect this. It current indicates that nodeId should be set for all three node transactions, where it is only actually set for NodeCreate

@SvetBorislavov
Copy link

+1

@jsync-swirlds jsync-swirlds self-assigned this Nov 15, 2024
@jsync-swirlds
Copy link
Member

HIP update is already in process; proto file update in both repositories will follow soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants