Skip to content

feat: Improve to_bytes/from_bytes serialization and deserialization#2390

Draft
manishdait wants to merge 11 commits into
hiero-ledger:mainfrom
manishdait:feat/improve-serialization-deserialization
Draft

feat: Improve to_bytes/from_bytes serialization and deserialization#2390
manishdait wants to merge 11 commits into
hiero-ledger:mainfrom
manishdait:feat/improve-serialization-deserialization

Conversation

@manishdait

@manishdait manishdait commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

Description:
This PR improve the to_bytes/from_bytes serialization and deserialization, to support parsing multiple node_account_ids in transaction.

Changes Made:

  • Refactor build_base_transaction() to create the transaction body without the transaction_id and node_account_id
  • Updated freeze_with() method to handle the node_account_ids list
  • Deprecated the use of node_account_id and set_node_account_id() to prefer the set_node_account_ids instead to match other sdks
  • Updated chunked transaction to build all the transaction bytes on freeze instead, of creating them on execute, and to help with serialization
  • Updated uint test for the changes

Related issue(s):

Fixes #2302

Notes for reviewer:

  • Most of the changes are internal that will not cause breaking changes, to prevent some breaking changes additional methods and properties are added. which are mark as deprecated
  • We can remove this deprecated method in future if we want.
  • Also added a _from_protobuf method only to AccountCreateTransaction for now to test changes. Since we have another PR that is currently working on adding the protobuf method to other transaction
  • Remove unit test for the to_bytes that previously depends on the require_frozen
  • Still in initial draft, need to verify and update some e2e/unit test

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@github-actions github-actions Bot added the skill: advanced requires knowledge of multiple areas in the codebase without defined steps to implement or examples label Jun 27, 2026
@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.13262% with 8 lines in your changes missing coverage. Please review.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2390      +/-   ##
==========================================
- Coverage   95.01%   95.01%   -0.01%     
==========================================
  Files         163      163              
  Lines       10465    10562      +97     
==========================================
+ Hits         9943    10035      +92     
- Misses        522      527       +5     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@manishdait manishdait added the notes: extra care requires extra attention to detail or requirements label Jul 6, 2026
@manishdait manishdait force-pushed the feat/improve-serialization-deserialization branch from 03e8ca7 to a4d0127 Compare July 6, 2026 20:59
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

Hi, this is WorkflowBot.
Your pull request cannot be merged as it is not passing all our workflow checks.
Please click on each check to review the logs and resolve issues so all checks pass.
To help you:

Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
@manishdait manishdait force-pushed the feat/improve-serialization-deserialization branch from a4d0127 to 2b2fc79 Compare July 7, 2026 08:56
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>

@aceppaluni aceppaluni left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some questions:

Should we consider restoring all temporary mutable state (such as _current_chunk_index) after helper methods that iterate through chunks?

return

if signed_transaction.sigMap.sigPair:
restored_transaction._signature_map[signed_transaction.bodyBytes] = signed_transaction.sigMap

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like the assumption here is bodyBytes uniquely identifies a transaction body. However, is it possible to have duplicate body bytes across different nodes?


if transaction_body.HasField("transactionID"):
transaction.transaction_id = TransactionId._from_proto(transaction_body.transactionID)
transaction._transaction_ids = [TransactionId._from_proto(transaction_body.transactionID)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For chunked transactions, is the ordering guaranteed?

@aceppaluni aceppaluni added the status: Needs Developer Revision Author needs to apply suggested changes/improvements label Jul 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

notes: extra care requires extra attention to detail or requirements skill: advanced requires knowledge of multiple areas in the codebase without defined steps to implement or examples status: Needs Developer Revision Author needs to apply suggested changes/improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update Transaction.to_bytes() / from_bytes() to support multi-node transactions using TransactionList

2 participants