swap order of event subscriptions in keystone keyring constructor #2204
Conversation
| this.getInteraction().emit(MemStoreDataReady, request); | ||
| } | ||
| }); | ||
| const data = this.getMemStore().getState(); |
There was a problem hiding this comment.
This commit doesn't work for me. Is that working for you?
There was a problem hiding this comment.
No, I did some more testing and ran into some issues that I'm trying to work out. I think I don't quite understand the event interactions well enough.
There was a problem hiding this comment.
#2202 This fix doesn't work for you?
There was a problem hiding this comment.
No it doesn't. That change fixes it so that the first signing attempt is valid, but afterwards it starts sending old transactions to the wallet.
I pushed some more changes to this branch that seem to resolve the issue (without the changes from #2022 even).
There was a problem hiding this comment.
Signed transactions require a 100ms delay after the call to wait for the UI to be mounted.
(Fixed in the previous version https://github.com/RabbyHub/Rabby/pull/2192/files#diff-8b4639172afdc1cf22ddb09ec5ef9d896214d458eacab485742333ec0dd668dbR443)
So the bug with keystone is that getting the data comes before signing the request.(https://github.com/RabbyHub/Rabby/blob/develop/src/background/controller/wallet.ts#L2564-L2572)
So each time the UI gets the same result as the previous one.
Maybe we just need to subscribe to the memStore changes and then call MemStoreDataReady to pass the data.
| this.getInteraction().on(AcquireMemeStoreData, () => { | ||
| const request = this.getMemStore().getState().sign?.request; | ||
| if (request) { | ||
| this.getInteraction().emit(MemStoreDataReady, request); | ||
| } |
There was a problem hiding this comment.
| this.getInteraction().on(AcquireMemeStoreData, () => { | |
| const request = this.getMemStore().getState().sign?.request; | |
| if (request) { | |
| this.getInteraction().emit(MemStoreDataReady, request); | |
| } |
|
#2202 seems to resolve the issue |
|
apparently not. It seemed to work better but after some testing I still managed to end up getting a stale transaction request instead of the one I wanted using only #2202. Disabling the |
|
Okay it might be working and I've now conflated this bug with #2209. I'll keep testing the latest release. |
calling 'getMemStore().subscribe' before the AcquireMemeStoreData event is fired caused the 'request' variable to be stale
fixes #2203