Skip to content

refactor: split nano contract send method into create and mine/push #660

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

Merged
merged 2 commits into from
Jun 10, 2024

Conversation

pedroferreira1
Copy link
Member

Motivation

The desktop wallet has a flow for sending tx that it expects the SendTransaction object because internally we handle the states of mine/push with the error handling and UI feedback for the user.

Acceptance Criteria

  • Transform the create and send nano contract method into two, (i) build tx and create SendTransaction object and (ii) create, mine, and push.

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@pedroferreira1 pedroferreira1 requested review from tuliomir and r4mmer June 6, 2024 04:12
@pedroferreira1 pedroferreira1 self-assigned this Jun 6, 2024
Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.71%. Comparing base (0b7338d) to head (fa1b453).

Additional details and impacted files
@@             Coverage Diff             @@
##           master     #660       +/-   ##
===========================================
+ Coverage   62.61%   79.71%   +17.09%     
===========================================
  Files          77       77               
  Lines        5856     5857        +1     
  Branches     1217     1217               
===========================================
+ Hits         3667     4669     +1002     
+ Misses       2093     1171      -922     
+ Partials       96       17       -79     

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

@pedroferreira1 pedroferreira1 force-pushed the refactor/split-send-nano-tx-method branch from 2abc5ba to 5163609 Compare June 6, 2024 14:07
r4mmer
r4mmer previously approved these changes Jun 6, 2024
*
* @param tx Transaction to sign and send
* @param pin Pin to decrypt data
* @param storage Wallet storage object
*/
export const signAndPushNCTransaction = async (tx: NanoContract, pin: string, storage: IStorage): Promise<Transaction> => {
export const signAndCreateSendTransaction = async (tx: NanoContract, pin: string, storage: IStorage): Promise<SendTransaction> => {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion(non-blocking): rename to prepareNanoSendTransaction

While the body of the function it is not nano specific the typing indicates that this is to be used with a nano contract transaction, maybe we should reflect this in the name as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense! Done bb3c613

tuliomir
tuliomir previously approved these changes Jun 6, 2024
@pedroferreira1 pedroferreira1 dismissed stale reviews from tuliomir and r4mmer via bb3c613 June 10, 2024 16:40
@pedroferreira1 pedroferreira1 force-pushed the refactor/split-send-nano-tx-method branch from bb3c613 to fa1b453 Compare June 10, 2024 16:41
@pedroferreira1 pedroferreira1 merged commit 1c63177 into master Jun 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants