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

Get rid of transactions queue #1027

Closed
flexsurfer opened this Issue Jun 12, 2018 · 7 comments

Comments

Projects
None yet
3 participants
@flexsurfer
Member

flexsurfer commented Jun 12, 2018

Problem

Because we don't have "sign later" functionality anymore, there is no reason to have transactions queue on go side.

Implementation

-remove transactions queue functionality
-instead two approve methods implement one sendTransaction

image

@mandrigin

This comment has been minimized.

Member

mandrigin commented Jun 21, 2018

@flexsurfer but we still need to signal to react side that a DApp tries to send a transaction, right? or, how that would be implemented?

@mandrigin

This comment has been minimized.

Member

mandrigin commented Jun 21, 2018

even if they just do any raw call...
but then we are moving all the protection on the level of web3.js that is hardly maintained

@flexsurfer

This comment has been minimized.

Member

flexsurfer commented Jun 21, 2018

@mandrigin so how it works now, we have stutusHTTPProvider, which has send and sendAsync https://github.com/status-im/status-react/blob/develop/resources/js/web3_init.js#L56, where we call status-go and wait signal, i suggest to do not call status-go, but show wallet UI, and then if user presses send, call status-go, so just remove queue and signals

@mandrigin

This comment has been minimized.

Member

mandrigin commented Jun 21, 2018

ah, ok, so that would be a part of status-react now. makes sense.
same for personal_sign and the other stuff, I guess, right?

--
note to whoever will implement that, that we will still need to sign everything locally and use the local nonce and stuff, these things aren't for removal.

@flexsurfer

This comment has been minimized.

Member

flexsurfer commented Jun 21, 2018

right, thanks

@flexsurfer

This comment has been minimized.

Member

flexsurfer commented Jul 26, 2018

@sebastiandelgado any updates on this? what are our next steps?

@sebastiandelgado

This comment has been minimized.

Contributor

sebastiandelgado commented Jul 26, 2018

@flexsurfer Its almost done. Already approved and merged most of the changes into the branch feature/remove-transactions-queue-1027. I have a PR open for the last commit (#1117) that should make this feature branch ready to merge. You can check the new interface for calling signMessage and sendTransaction at the lib/library.go file.

adambabik added a commit that referenced this issue Aug 16, 2018

Remove transactions queue 1027 (#1125)
Remove `PendingSignRequests` queue from the sign module.

This closes #1027 by removing the pending sign requests queue dependency from the SendTransaction, SignMessage and Recover.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment