Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Simplify getBytes and getSigningBytes #9062

Merged
merged 3 commits into from
Nov 1, 2023
Merged

Conversation

bobanm
Copy link
Contributor

@bobanm bobanm commented Oct 4, 2023

What was the problem?

This PR resolves #9061

How was it solved?

  • Extracted common functionality to encodeParams() function
  • Reverted the conditional for checking if params are valid
  • Updated validateTransaction() to throws error instead of returning an error
  • Updated encodeParams() to default params schema to emptySchema

How was it tested?

All existing tests have passed 👌🏻

@bobanm bobanm requested a review from shuse2 October 4, 2023 04:20
@bobanm bobanm self-assigned this Oct 4, 2023
@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #9062 (5cf34e9) into development (773ba74) will decrease coverage by 0.90%.
Report is 2 commits behind head on development.
The diff coverage is 92.85%.

❗ Current head 5cf34e9 differs from pull request most recent head 7608396. Consider uploading reports for the commit 7608396 to get more accurate results

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #9062      +/-   ##
===============================================
- Coverage        84.35%   83.46%   -0.90%     
===============================================
  Files              655      604      -51     
  Lines            24137    22776    -1361     
  Branches          3506     3356     -150     
===============================================
- Hits             20361    19010    -1351     
+ Misses            3776     3766      -10     
Files Coverage Δ
elements/lisk-transactions/src/validate.ts 100.00% <100.00%> (ø)
elements/lisk-transactions/src/sign.ts 98.07% <94.44%> (-1.93%) ⬇️
framework/src/testing/create_transaction.ts 20.83% <0.00%> (+1.60%) ⬆️

... and 103 files with indirect coverage changes

elements/lisk-transactions/src/sign.ts Outdated Show resolved Hide resolved
elements/lisk-transactions/src/sign.ts Outdated Show resolved Hide resolved
@bobanm bobanm requested a review from shuse2 October 19, 2023 00:35
@bobanm bobanm force-pushed the 9061-simplify-getBytes branch 2 times, most recently from 05358df to 5cf34e9 Compare October 19, 2023 00:58
@bobanm bobanm changed the base branch from release/6.0.0 to development October 20, 2023 10:46
@bobanm bobanm marked this pull request as ready for review October 20, 2023 10:47
@bobanm bobanm requested a review from has5aan October 20, 2023 10:48
@shuse2 shuse2 enabled auto-merge November 1, 2023 16:38
@shuse2 shuse2 merged commit d29adbc into development Nov 1, 2023
9 checks passed
@shuse2 shuse2 deleted the 9061-simplify-getBytes branch November 1, 2023 17:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify getBytes and getSigningBytes functions of lisk-transactions
3 participants