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

fix: serialize deserialize FileAppend #2532

Merged
merged 29 commits into from
Oct 4, 2024

Conversation

ivaylonikolov7
Copy link
Contributor

@ivaylonikolov7 ivaylonikolov7 commented Sep 18, 2024

Description:
Implement chunk logic for toBytes method and disallow addSignature function until the next release.

Fixes #2167 #2536

Notes for reviewer:
There should be refactoring done. We need to implement ChunkTransaction. This fix is more like a patch before the major refactor for ChunkedTransaction like the other SDKs.

Checklist

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

@ivaylonikolov7 ivaylonikolov7 marked this pull request as ready for review September 19, 2024 14:24
@ivaylonikolov7 ivaylonikolov7 requested review from a team as code owners September 19, 2024 14:24
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 82.05128% with 21 lines in your changes missing coverage. Please review.

Project coverage is 84.39%. Comparing base (8bf5b24) to head (a4fa3a7).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/file/FileAppendTransaction.js 77.89% 21 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2532   +/-   ##
=======================================
  Coverage   84.39%   84.39%           
=======================================
  Files         283      283           
  Lines       70929    71017   +88     
=======================================
+ Hits        59863    59938   +75     
- Misses      11066    11079   +13     

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

src/file/FileAppendTransaction.js Outdated Show resolved Hide resolved
test/integration/FileAppendIntegrationTest.js Outdated Show resolved Hide resolved
test/integration/FileAppendIntegrationTest.js Outdated Show resolved Hide resolved
test/integration/FileAppendIntegrationTest.js Outdated Show resolved Hide resolved
0xivanov
0xivanov previously approved these changes Sep 27, 2024
Copy link
Contributor

@0xivanov 0xivanov left a comment

Choose a reason for hiding this comment

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

LGTM

test/integration/FileAppendIntegrationTest.js Show resolved Hide resolved
src/file/FileAppendTransaction.js Outdated Show resolved Hide resolved
test/integration/FileAppendIntegrationTest.js Show resolved Hide resolved
src/file/FileAppendTransaction.js Outdated Show resolved Hide resolved
src/transaction/Transaction.js Show resolved Hide resolved
Copy link
Contributor

@agadzhalov agadzhalov left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

sonarcloud bot commented Oct 4, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
13.7% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@ivaylonikolov7 ivaylonikolov7 merged commit 596d115 into main Oct 4, 2024
11 of 13 checks passed
@ivaylonikolov7 ivaylonikolov7 deleted the feat/serialize-deserialize-tx branch October 4, 2024 15:00
This was referenced Oct 7, 2024
b-l-u-e pushed a commit to b-l-u-e/hedera-sdk-js that referenced this pull request Oct 13, 2024
* fix: serialize deserialize fileappend

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: return _makeTransactionData

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: add integration test for fromByte toByte converison

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: implement better transaction conversion

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: fileappend hip-765 finish implementation

Signed-off-by: Ivaylo Nikolov <[email protected]>

* revert: remove  multi signature multi node feat for FileAppend

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: freeze transaction after fromBytes

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: disallow addSignature check for chunked transactions in transaction class

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: linter complains about skipped test

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: move type definitions because of linter

Signed-off-by: Ivaylo Nikolov <[email protected]>

* revert: old type definitions

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: throw error when required chunks are more than max chunks

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: required chunks should not be an error

Signed-off-by: Ivaylo Nikolov <[email protected]>

* style: fix jsdoc comment when building incomplete transaction

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: reuse variables in file append integration tests

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: delete already defined variables

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: fix typo in test name

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: return back addSignature return  type

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: fix typo

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: implement maxChunks when serializing content too

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: wrong comparison

Signed-off-by: Ivaylo Nikolov <[email protected]>

* style: add empty lines

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: generate new transactionId

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: add more integration tests

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: extract dummy account id to const

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: fix function nane typo

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: fix function name typo 2

Signed-off-by: Ivaylo Nikolov <[email protected]>

* chore: remove incompleted references

Signed-off-by: Ivaylo Nikolov <[email protected]>

---------

Signed-off-by: Ivaylo Nikolov <[email protected]>
Signed-off-by: b-l-u-e <[email protected]>
b-l-u-e pushed a commit to b-l-u-e/hedera-sdk-js that referenced this pull request Oct 13, 2024
* fix: serialize deserialize fileappend

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: return _makeTransactionData

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: add integration test for fromByte toByte converison

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: implement better transaction conversion

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: fileappend hip-765 finish implementation

Signed-off-by: Ivaylo Nikolov <[email protected]>

* revert: remove  multi signature multi node feat for FileAppend

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: freeze transaction after fromBytes

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: disallow addSignature check for chunked transactions in transaction class

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: linter complains about skipped test

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: move type definitions because of linter

Signed-off-by: Ivaylo Nikolov <[email protected]>

* revert: old type definitions

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: throw error when required chunks are more than max chunks

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: required chunks should not be an error

Signed-off-by: Ivaylo Nikolov <[email protected]>

* style: fix jsdoc comment when building incomplete transaction

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: reuse variables in file append integration tests

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: delete already defined variables

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: fix typo in test name

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: return back addSignature return  type

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: fix typo

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: implement maxChunks when serializing content too

Signed-off-by: Ivaylo Nikolov <[email protected]>

* fix: wrong comparison

Signed-off-by: Ivaylo Nikolov <[email protected]>

* style: add empty lines

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: generate new transactionId

Signed-off-by: Ivaylo Nikolov <[email protected]>

* test: add more integration tests

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: extract dummy account id to const

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: fix function nane typo

Signed-off-by: Ivaylo Nikolov <[email protected]>

* refactor: fix function name typo 2

Signed-off-by: Ivaylo Nikolov <[email protected]>

* chore: remove incompleted references

Signed-off-by: Ivaylo Nikolov <[email protected]>

---------

Signed-off-by: Ivaylo Nikolov <[email protected]>
Signed-off-by: b-l-u-e <[email protected]>
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 this pull request may close these issues.

FileAppendTransaction incorrect content after deserialization
4 participants