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

Signer CLI - send offline #79

Merged
merged 10 commits into from
Jan 10, 2020
Merged

Signer CLI - send offline #79

merged 10 commits into from
Jan 10, 2020

Conversation

ma12ki
Copy link
Contributor

@ma12ki ma12ki commented Jan 7, 2020

This addresses #74 - adds a new signer-cli command called sendOffline that prints the signed transaction instead of submitting it to the network.

It's basically just a PoC for now. If you find it valuable, I'll work on proper typing and documentation.

@ma12ki ma12ki mentioned this pull request Jan 7, 2020
Closed
jacogr
jacogr reviewed Jan 9, 2020
View reviewed changes
Copy link
Member

@jacogr jacogr left a comment •
edited
Loading

Choose a reason for hiding this comment

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

There is a def. a need for this. It would certainly be benificial to have this functionality.

@ma12ki
Copy link
Contributor Author

ma12ki commented Jan 9, 2020

Perfect. I'll work on making it pretty then.

@ma12ki
Copy link
Contributor Author

ma12ki commented Jan 10, 2020

OK, added types, documentation, some small improvements and you can now submit transactions generated with sendOffline using submit. Please take a look.

@jacogr
Copy link
Member

jacogr commented Jan 10, 2020

This looks good. I also think there is probably a case to be made for doing exactly this inline, ie. not having to copy stuff across. (However that is a bit more work since it break assumptions made initially, so will log that)

@jacogr jacogr merged commit 4796e4f into polkadot-js:master Jan 10, 2020
@ma12ki
Copy link
Contributor Author

ma12ki commented Jan 10, 2020

Nice! Yes I thought about the inline case as well but wanted to follow the initial assumptions as you say. Definitely something that could be added later. Thanks for merging this 🎆

@ma12ki ma12ki mentioned this pull request Jan 27, 2020
Merged
@polkadot-js-bot
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@polkadot-js polkadot-js locked as resolved and limited conversation to collaborators Jun 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jacogr jacogr jacogr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ma12ki @jacogr @polkadot-js-bot