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.

Sign up for GitHub

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

Jump to bottom

Enable users to receive and respond to transaction signature request - Closes #4418 #4460

Merged
merged 20 commits into from
Oct 31, 2022
Merged

Enable users to receive and respond to transaction signature request - Closes #4418 #4460

merged 20 commits into from
Oct 31, 2022

Conversation

reyraa
Copy link
Contributor

@reyraa reyraa commented Aug 31, 2022 •
edited
Loading

What was the problem?

This PR resolves #4418

How was it solved?

  • Implement tx signature logic
  • Ensure Desktop can handle different tx types
  • Add required unit tests

How was it tested?

  • Manual testing via different tx types
  • Unit tests

@reyraa reyraa self-assigned this Aug 31, 2022
@reyraa reyraa mentioned this pull request Sep 1, 2022
Merged
5 tasks
@ManuGowda
Copy link
Contributor

This PR is suspended until we resolve transaction signing related issues in SDK v6 objective.

CC: @soroushm tagging you so that next time onwards an PRs which gets blocked like this we should mark them suspend and leave a comment, please inform team members to do, if failed please take care of it yourself 🙏🏼

Marking suspend of providing appropriate labels help the auditor understand the reasoning behind our actions as well as within and across our own departments 🙏🏼

@reyraa reyraa marked this pull request as ready for review October 21, 2022 07:09
@reyraa reyraa changed the base branch from feature/transaction-signing to development October 21, 2022 07:27
@reyraa reyraa changed the base branch from development to feature/transaction-signing October 21, 2022 07:27
@reyraa reyraa force-pushed the 4418-implement-tx-signature-logic branch from f4e7a58 to 763660e Compare October 21, 2022 07:54
@reyraa reyraa force-pushed the 4418-implement-tx-signature-logic branch from 763660e to 51844c3 Compare October 21, 2022 07:55
@sridharmeganathan sridharmeganathan requested review from soroushm and removed request for ManuGowda October 24, 2022 07:36
ikem-legend
ikem-legend reviewed Oct 24, 2022
View reviewed changes
libs/wcm/utils/connectionCreator.js Outdated
projectId: process.env.PROJECT_ID,
relayUrl: process.env.RELAY_URL,
// relayUrl: process.env.RELAY_URL,
Copy link
Member

Choose a reason for hiding this comment

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

@reyraa Commented code can either be removed or have a comment to explain the reason for it

soroushm
soroushm reviewed Oct 24, 2022
View reviewed changes
Copy link
Contributor

@soroushm soroushm left a comment

Choose a reason for hiding this comment

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

init

libs/wcm/utils/connectionCreator.js
projectId: process.env.PROJECT_ID,
relayUrl: process.env.RELAY_URL,
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please move it to config.js

Copy link
Contributor Author

@reyraa reyraa Oct 30, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Good point. Since i'm gonna apply a few changes with #4482 so i can test it with Lisk Mobile too. Is that alright?

src/modules/account/components/AddAccountByFile/AddAccountByFile.js Outdated Show resolved Hide resolved
src/modules/blockchainApplication/connection/components/RequestSummary/index.js Show resolved Hide resolved
src/modules/blockchainApplication/connection/components/RequestSummary/index.js Show resolved Hide resolved
soroushm
soroushm approved these changes Oct 31, 2022
View reviewed changes
Copy link
Contributor

@soroushm soroushm left a comment

Choose a reason for hiding this comment

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

LGTM

@reyraa reyraa requested a review from ikem-legend October 31, 2022 08:35
@soroushm soroushm merged commit 26b86a1 into feature/transaction-signing Oct 31, 2022
@soroushm soroushm deleted the 4418-implement-tx-signature-logic branch October 31, 2022 08:36
@reyraa reyraa removed the request for review from ikem-legend October 31, 2022 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ikem-legend ikem-legend ikem-legend left review comments

@soroushm soroushm soroushm approved these changes

Assignees

@reyraa reyraa

Labels
None yet
Projects
No open projects
Lisk Desktop Version 3.0.0
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@reyraa @ManuGowda @soroushm @ikem-legend