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

feat: Add removeSignature/clearAllSignatures methods #2491

Merged
merged 16 commits into from
Sep 20, 2024

Conversation

ivaylogarnev-limechain
Copy link
Contributor

Description:
This PR introduces the removeSignature and clearAllSignatures methods, allowing transactions to manage signatures more flexibly. Specifically, these methods enable the removal of individual signatures or the clearing of all signatures from a transaction after they have been added and signed.

Related issue(s):
#2461

Checklist

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

… class, wrote some unit tests for both

Signed-off-by: ivaylogarnev-limechain <[email protected]>
@ivaylogarnev-limechain ivaylogarnev-limechain force-pushed the feat/2461-remove-clear-signatures branch from 6607bd9 to 0828c1d Compare August 27, 2024 06:33
…d throwing an error if trying to remove non-existing signature

Signed-off-by: ivaylogarnev-limechain <[email protected]>
@ivaylogarnev-limechain ivaylogarnev-limechain force-pushed the feat/2461-remove-clear-signatures branch 2 times, most recently from 20136c7 to 1ecb4ed Compare August 28, 2024 08:13
@ivaylogarnev-limechain ivaylogarnev-limechain force-pushed the feat/2461-remove-clear-signatures branch from 1ecb4ed to af34748 Compare August 28, 2024 08:17
…movedSignatures and wrote tests for both

Signed-off-by: ivaylogarnev-limechain <[email protected]>
src/transaction/Transaction.js Show resolved Hide resolved
test/unit/Transaction.js Show resolved Hide resolved
… adjusted the logic, renamed clearAllSignatures and changed the return type, adjusted some tests

Signed-off-by: ivaylogarnev-limechain <[email protected]>
@ivaylogarnev-limechain ivaylogarnev-limechain force-pushed the feat/2461-remove-clear-signatures branch from e4652ea to 10525e9 Compare September 19, 2024 19:04
@ivaylogarnev-limechain ivaylogarnev-limechain force-pushed the feat/2461-remove-clear-signatures branch from 10525e9 to 81245d0 Compare September 19, 2024 19:05
ivaylonikolov7
ivaylonikolov7 previously approved these changes Sep 20, 2024
agadzhalov
agadzhalov previously approved these changes Sep 20, 2024
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

…aph/hedera-sdk-js into feat/2461-remove-clear-signatures

Signed-off-by: ivaylogarnev-limechain <[email protected]>
@ivaylogarnev-limechain ivaylogarnev-limechain force-pushed the feat/2461-remove-clear-signatures branch from a79edd5 to c5e7d1e Compare September 20, 2024 10:59
Copy link

sonarcloud bot commented Sep 20, 2024

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarCloud

@ivaylogarnev-limechain ivaylogarnev-limechain merged commit e03746e into main Sep 20, 2024
6 of 7 checks passed
@ivaylogarnev-limechain ivaylogarnev-limechain deleted the feat/2461-remove-clear-signatures branch September 20, 2024 11:59
b-l-u-e pushed a commit to b-l-u-e/hedera-sdk-js that referenced this pull request Oct 13, 2024
* feat: Added removeSignature/clearAllSignatures methods to Transaction class, wrote some unit tests for both

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* feat: Added more unit tests for removeSignature/clearAllSignatures and throwing an error if trying to remove non-existing signature

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Turned signAndAddSignatures function into arrow

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Cleared some unnecessary comments and changed variable assigment

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* feat: Enhancement removeSignature/clearAllSignatures to return the removedSignatures and wrote tests for both

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Removed the signature param from removeSignature method and adjusted the logic, renamed clearAllSignatures and changed the return type, adjusted some tests

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Remove redundant code

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* Merge branch 'main' into feat/2461-remove-clear-signatures

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Cleared some uneccessary comments & changed a bit the flow some methods conditions

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* Merge branch main into feat/2461-remove-clear-signatures

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Refactored signTransaction method a bit, added integration tests and examples for the multi-node flow

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Removed duplicate integration tests and added description to examples

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Examples pre-release warnings fixes

Signed-off-by: ivaylogarnev-limechain <[email protected]>

---------

Signed-off-by: ivaylogarnev-limechain <[email protected]>
Signed-off-by: ivaylogarnev <[email protected]>
Co-authored-by: ivaylonikolov7 <[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
* feat: Added removeSignature/clearAllSignatures methods to Transaction class, wrote some unit tests for both

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* feat: Added more unit tests for removeSignature/clearAllSignatures and throwing an error if trying to remove non-existing signature

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Turned signAndAddSignatures function into arrow

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Cleared some unnecessary comments and changed variable assigment

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* feat: Enhancement removeSignature/clearAllSignatures to return the removedSignatures and wrote tests for both

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Removed the signature param from removeSignature method and adjusted the logic, renamed clearAllSignatures and changed the return type, adjusted some tests

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Remove redundant code

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* Merge branch 'main' into feat/2461-remove-clear-signatures

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Cleared some uneccessary comments & changed a bit the flow some methods conditions

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* Merge branch main into feat/2461-remove-clear-signatures

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Refactored signTransaction method a bit, added integration tests and examples for the multi-node flow

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Removed duplicate integration tests and added description to examples

Signed-off-by: ivaylogarnev-limechain <[email protected]>

* refactor: Examples pre-release warnings fixes

Signed-off-by: ivaylogarnev-limechain <[email protected]>

---------

Signed-off-by: ivaylogarnev-limechain <[email protected]>
Signed-off-by: ivaylogarnev <[email protected]>
Co-authored-by: ivaylonikolov7 <[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.

Add functions for remove/clear signatures
5 participants