feat: Improve to_bytes/from_bytes serialization and deserialization#2390
feat: Improve to_bytes/from_bytes serialization and deserialization#2390manishdait wants to merge 11 commits into
Conversation
Codecov Report❌ Patch coverage is @@ 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:
|
03e8ca7 to
a4d0127
Compare
|
Hi, this is WorkflowBot.
|
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>
a4d0127 to
2b2fc79
Compare
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
Signed-off-by: Manish Dait <daitmanish88@gmail.com>
aceppaluni
left a comment
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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)] |
There was a problem hiding this comment.
For chunked transactions, is the ordering guaranteed?
Description:
This PR improve the
to_bytes/from_bytesserialization and deserialization, to support parsing multiplenode_account_idsin transaction.Changes Made:
build_base_transaction()to create the transaction body without thetransaction_idandnode_account_idfreeze_with()method to handle thenode_account_idslistnode_account_idandset_node_account_id()to prefer theset_node_account_idsinstead to match other sdksRelated issue(s):
Fixes #2302
Notes for reviewer:
_from_protobufmethod only toAccountCreateTransactionfor now to test changes. Since we have another PR that is currently working on adding the protobuf method to other transactionto_bytesthat previously depends on therequire_frozenChecklist