feat(wallet) integrate Wallet Connect sign APIs #12716
Conversation
Jenkins BuildsClick to see older builds (19)
|
stefandunca
force-pushed
the
wc_sign_api-12637
branch
3 times, most recently
from
6360bbd
to
7236701
Compare
stefandunca
requested review from
saledjenic,
IvanBelyakoff,
dlipicar and
Khushboo-dev-cpp
| } | ||
| } | ||
| ) | ||
| eventsTimer.start() |
There was a problem hiding this comment.
@stefandunca I don't see that we ever stop this timer, probably that was your intention to check every 100ms for a change?
There was a problem hiding this comment.
Indeed. We start it after pairing and never stop until the WalletConnectSDK is unloaded. When we add a un-pair/disconnect we will stop the timer then.
| function requestSdkAsync(jsCode) { | ||
| root.runJavaScript(jsCode, | ||
| function(result) { | ||
| timer.restart() |
There was a problem hiding this comment.
@stefandunca why don't we create and call for example "handleResponse(result)" function from here instead of running the timer? Guess that the result here is the same as wcResult used in onTriggered slot of the timer? The same question is for the same part in requestSdk function.
There was a problem hiding this comment.
This is due to the polling mechanism that we use to track the result of async operations running inside WebView. The handler here is returning after the async operation was started and timer will check regularly for the answer then stop. I'm working to improve this part and make it more robust. Still, the only option we have now is to communicate between QML runtime and WebView runtime without creating a client/server approach which is unsafe.
| function startListeningForEvents() { | ||
| const jsCode = " | ||
| try { | ||
| async function processWCEvents() { |
There was a problem hiding this comment.
@stefandunca why is this async?
There was a problem hiding this comment.
It's a recommended way of running calls without blocking the WebView runtime thread. For this specific call, it is optional; however, I plan to improve on this with my current work, which will enhance the robustness of the webview-qml communication.
| disconnect: function (topic) { | ||
| return window.wc.core.pairing.disconnect({ topic: topic}); | ||
| }, | ||
|
|
||
| registerForSessionRequest: function (callback) { | ||
| window.wc.web3wallet.on("session_request", callback); |
There was a problem hiding this comment.
@stefandunca is it safe to call many many times this? Like does web3wallet handle multiple listeners for the same event with or without overwriting?
There was a problem hiding this comment.
registerForSessionRequest is called only once after SDK is initialized. Will improve though, with my current work.
| } | ||
| window.wcEventError" | ||
|
|
||
| root.runJavaScript(jsCode, |
There was a problem hiding this comment.
@stefandunca why is it not enough to call jsCode only once and then check in a loop (using a timer) for window.wcEventResult instead of calling registerForSessionRequest every time?
There was a problem hiding this comment.
The call startListeningForEvents which executes registerForSessionRequest is called only once after the SDK initializes.
stefandunca
force-pushed
the
wc_pair_api-12551
branch
from
59e4773
to
2e472c5
Compare
stefandunca
force-pushed
the
wc_sign_api-12637
branch
from
7236701
to
47f546c
Compare
| @@ -24,7 +25,7 @@ window.wc = { | |||
| metadata: { | |||
| name: "Status", | |||
| description: "Status Wallet", | |||
| url: "https://status.app", | |||
| url: "http://localhost", | |||
There was a problem hiding this comment.
It is temporary or leftover?
There was a problem hiding this comment.
It is a fix for MacOS DMG sand-boxing generating a warning that the status.app is not allowed by the signing. We don't do a request to any domain so we just silence the warning.
stefandunca
force-pushed
the
wc_sign_api-12637
branch
from
47f546c
to
743ca0f
Compare
Bump status-go that brings the sign APIs support for send transaction and personal sign Extend SDK - simple SDK event handling in QML - support session request response APIs - pairing management Closes #12637
stefandunca
force-pushed
the
wc_sign_api-12637
branch
from
743ca0f
to
1024a4d
Compare
Closes #12637
The current
WebView-QMLcommunication is at a prototype stage to show that the integration works. The current work to make it robust is subsequent with this work package: #12639Bump
status-goto #4279Extend SDK