-
Notifications
You must be signed in to change notification settings - Fork 208
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
feat(wallet-connection): Connect dapp directly to wallet UI #5750
Changes from all commits
23397b2
16b153a
6f84447
9a15c3e
ce70a36
99e920d
b17d4b7
0110b47
90b89df
49ad042
3e8e76d
454e918
01fa1d6
17eb1c8
bd23daa
1cf29d7
688cc2b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -154,11 +154,16 @@ | |||||||||
}); | ||||||||||
}; | ||||||||||
|
||||||||||
// Start the flow of messages. | ||||||||||
if (window.parent !== window) { | ||||||||||
window.parent.postMessage({ type: 'walletBridgeLoaded' }, '*'); | ||||||||||
} | ||||||||||
retryWebSocket(); | ||||||||||
// This ensures the message wont be posted until after the iframe's | ||||||||||
// "onLoad" event fires so we can rely on the consistent ordering of | ||||||||||
// events in the WalletConnection component. | ||||||||||
window.addEventListener('load', () => { | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. your PR comment would make a good code comment.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yes, please! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||||||||||
// Start the flow of messages. | ||||||||||
if (window.parent !== window) { | ||||||||||
window.parent.postMessage({ type: 'walletBridgeLoaded' }, '*'); | ||||||||||
} | ||||||||||
retryWebSocket(); | ||||||||||
}); | ||||||||||
</script> | ||||||||||
</body> | ||||||||||
</html> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,52 @@ | ||
// cribbed from https://www.npmjs.com/package/react-app-rewire-multiple-entry | ||
samsiegart marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/* global __dirname, require, module */ | ||
|
||
const path = require('path'); | ||
const HtmlWebPackPlugin = require('html-webpack-plugin'); | ||
const multiEntry = require('react-app-rewire-multiple-entry'); | ||
|
||
const bridgeTemplate = path.resolve( | ||
path.join(__dirname, '..', 'public', 'bridge.html'), | ||
); | ||
|
||
const multipleEntry = multiEntry([ | ||
{ | ||
template: 'public/index.html', | ||
entry: 'src/index.jsx', | ||
}, | ||
{ | ||
template: 'public/bridge.html', | ||
entry: 'src/bridge.jsx', | ||
}, | ||
]); | ||
|
||
module.exports = function override(config, _env) { | ||
config.resolve.fallback = { path: false, crypto: false }; | ||
config.ignoreWarnings = [/Failed to parse source map/]; | ||
|
||
const htmlWebpackPlugin = config.plugins.find( | ||
plugin => plugin.constructor.name === 'HtmlWebpackPlugin', | ||
); | ||
if (!htmlWebpackPlugin) { | ||
throw new Error("Can't find HtmlWebpackPlugin"); | ||
} | ||
|
||
multipleEntry.addMultiEntry(config); | ||
|
||
const bridgeKeys = Object.keys(config.entry).filter(k => | ||
k.startsWith('bridge'), | ||
); | ||
const opts = { | ||
...htmlWebpackPlugin.userOptions, | ||
template: bridgeTemplate, | ||
filename: './bridge.html', | ||
chunks: bridgeKeys, | ||
}; | ||
htmlWebpackPlugin.userOptions = { | ||
...htmlWebpackPlugin.userOptions, | ||
excludeChunks: bridgeKeys, | ||
}; | ||
const plug2 = new HtmlWebPackPlugin(opts); | ||
config.plugins.push(plug2); | ||
return config; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
{ | ||
"compilerOptions": { | ||
"jsx": "react", | ||
"target": "esnext", | ||
"module": "esnext", | ||
"noEmit": true, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<!DOCTYPE html> | ||
<html lang="en"> | ||
|
||
<head> | ||
<meta charset="utf-8" /> | ||
<script src="%PUBLIC_URL%/lockdown.umd.js"></script> | ||
<title>Agoric Wallet Bridge</title> | ||
</head> | ||
|
||
<body style="overflow-x: hidden; overflow-y: scroll"> | ||
<noscript>You need to enable JavaScript to run this app.</noscript> | ||
<div id="root"></div> | ||
</body> | ||
|
||
</html> |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,135 @@ | ||||||||||||||||||||||
// @ts-check | ||||||||||||||||||||||
/* eslint-disable import/no-extraneous-dependencies */ | ||||||||||||||||||||||
import '@endo/eventual-send/shim'; | ||||||||||||||||||||||
import './lockdown.js'; | ||||||||||||||||||||||
|
||||||||||||||||||||||
import React from 'react'; | ||||||||||||||||||||||
import ReactDOM from 'react-dom'; | ||||||||||||||||||||||
|
||||||||||||||||||||||
Error.stackTraceLimit = Infinity; | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TIL. How do we know this is necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know. I looked around for clues without much luck. It does merit a comment, at least to point to some discussion of why we do this. Help, @kriskowal ? @erights ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can leave this convo open but I won't block this PR on it since it's preexisting code. |
||||||||||||||||||||||
|
||||||||||||||||||||||
/** ISSUE: where are these defined? */ | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ISSUE like file an issue? In master they're magic strings. I see two posts and one receive that's just a validation:
agoric-sdk/packages/web-components/src/bridge-iframe-connector.js Lines 20 to 27 in 7e16cc0
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Issue like: decide between us, whether to sweep it under the rug, address it, or file an issue. Yes, the magic strings pre-date this PR. Is it time to fix them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. looks to me that this PR solves the problem already. the question "where are these defined?" is answered and they're no longer magic. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the other occurrences don't refer to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ah, I thought this PR replaced the occurrences I linked to. ok, unresolved issue. can be punted imo. this whole package needs a types cleanup |
||||||||||||||||||||||
const BridgeProtocol = /** @type {const} */ ({ | ||||||||||||||||||||||
loaded: 'walletBridgeLoaded', | ||||||||||||||||||||||
opened: 'walletBridgeOpened', | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
|
||||||||||||||||||||||
const checkParentWindow = () => { | ||||||||||||||||||||||
const me = window; | ||||||||||||||||||||||
const { parent } = window; | ||||||||||||||||||||||
if (me === parent) { | ||||||||||||||||||||||
throw Error('window.parent === parent!!!'); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}; | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** | ||||||||||||||||||||||
* Install a dApp "connection" where messages posted from | ||||||||||||||||||||||
* the dApp are forwarded to localStorage and vice versa. | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* @param {{ | ||||||||||||||||||||||
* addEventListener: typeof window.addEventListener, | ||||||||||||||||||||||
* parentPost: typeof window.postMessage, | ||||||||||||||||||||||
* t0: number, | ||||||||||||||||||||||
* }} io | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
const installDappConnection = ({ addEventListener, parentPost, t0 }) => { | ||||||||||||||||||||||
checkParentWindow(); | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** @type { string } */ | ||||||||||||||||||||||
let origin; | ||||||||||||||||||||||
|
||||||||||||||||||||||
/** @type {(key: string, value: string) => void} */ | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised ts-check isn't alerting that Oh, there's no type checking in this package yet. #5757 |
||||||||||||||||||||||
let setItem; | ||||||||||||||||||||||
/** @type {string[]} */ | ||||||||||||||||||||||
const buffer = []; | ||||||||||||||||||||||
|
||||||||||||||||||||||
console.debug('bridge: installDappConnection'); | ||||||||||||||||||||||
|
||||||||||||||||||||||
parentPost({ type: BridgeProtocol.loaded }, '*'); | ||||||||||||||||||||||
parentPost({ type: BridgeProtocol.opened }, '*'); | ||||||||||||||||||||||
Comment on lines
+48
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no distinction anymore? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know. It feels a bit magic, to me. In There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, walletBridgeOpened doesn't seem to be read anywhere. Maybe useful for debugging, but only walletBridgeLoaded has any logic dependent on it. |
||||||||||||||||||||||
|
||||||||||||||||||||||
let outIx = 0; // TODO: persist index in storage | ||||||||||||||||||||||
/** @param {Record<string, any>} payload */ | ||||||||||||||||||||||
|
||||||||||||||||||||||
const { stringify, parse } = JSON; | ||||||||||||||||||||||
|
||||||||||||||||||||||
addEventListener('message', ev => { | ||||||||||||||||||||||
if (!ev.data?.type?.startsWith('CTP_')) { | ||||||||||||||||||||||
return; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
if (origin === undefined) { | ||||||||||||||||||||||
// First-come, first-serve. | ||||||||||||||||||||||
origin = ev.origin; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
if (setItem) { | ||||||||||||||||||||||
console.debug('bridge: message from dapp->localStorage', origin, ev.data); | ||||||||||||||||||||||
setItem(stringify(['out', origin, t0, outIx]), stringify(ev.data)); | ||||||||||||||||||||||
} else { | ||||||||||||||||||||||
console.debug('bridge: message from dapp->buffer', origin, ev.data); | ||||||||||||||||||||||
buffer.push(harden(ev.data)); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
|
||||||||||||||||||||||
return harden({ | ||||||||||||||||||||||
/** @param {typeof window.localStorage} storage */ | ||||||||||||||||||||||
connectStorage: storage => { | ||||||||||||||||||||||
addEventListener('storage', ev => { | ||||||||||||||||||||||
if (!ev.key || !ev.newValue) { | ||||||||||||||||||||||
return; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
const [tag, targetOrigin, targetT, _keyIx] = JSON.parse(ev.key); // or throw | ||||||||||||||||||||||
if (tag !== 'in' || targetOrigin !== origin || targetT !== t0) { | ||||||||||||||||||||||
return; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
const payload = parse(ev.newValue); // or throw | ||||||||||||||||||||||
storage.removeItem(ev.key); | ||||||||||||||||||||||
console.debug('bridge: storage -> message to dapp', origin, payload); | ||||||||||||||||||||||
parentPost(payload, '*'); | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
setItem = (key, value) => { | ||||||||||||||||||||||
storage.setItem(key, value); | ||||||||||||||||||||||
outIx += 1; | ||||||||||||||||||||||
}; | ||||||||||||||||||||||
if (buffer.length) { | ||||||||||||||||||||||
console.debug('sending', buffer.length, 'queued messages from', origin); | ||||||||||||||||||||||
while (buffer.length) { | ||||||||||||||||||||||
setItem( | ||||||||||||||||||||||
stringify(['out', origin, t0, outIx]), | ||||||||||||||||||||||
stringify(buffer.shift()), | ||||||||||||||||||||||
); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
}, | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
}; | ||||||||||||||||||||||
|
||||||||||||||||||||||
const conn = installDappConnection({ | ||||||||||||||||||||||
addEventListener: window.addEventListener, | ||||||||||||||||||||||
parentPost: (payload, origin) => window.parent.postMessage(payload, origin), | ||||||||||||||||||||||
t0: Date.now(), | ||||||||||||||||||||||
}); | ||||||||||||||||||||||
|
||||||||||||||||||||||
const signalBridge = () => { | ||||||||||||||||||||||
if ('requestStorageAccess' in document) { | ||||||||||||||||||||||
document | ||||||||||||||||||||||
.requestStorageAccess() | ||||||||||||||||||||||
.then(() => { | ||||||||||||||||||||||
console.debug('bridge: storage access granted'); | ||||||||||||||||||||||
conn.connectStorage(window.localStorage); | ||||||||||||||||||||||
}) | ||||||||||||||||||||||
.catch(whyNot => | ||||||||||||||||||||||
console.error('bridge: requestStorageAccess rejected with', whyNot), | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what's the UX in this case? worth an alert? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm actually not sure, |
||||||||||||||||||||||
); | ||||||||||||||||||||||
} else { | ||||||||||||||||||||||
console.debug('bridge: SKIP document.requestStorageAccess'); | ||||||||||||||||||||||
conn.connectStorage(window.localStorage); | ||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could this fail? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, I don't think so. It only calls There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've been bitten by |
||||||||||||||||||||||
} | ||||||||||||||||||||||
}; | ||||||||||||||||||||||
|
||||||||||||||||||||||
ReactDOM.render( | ||||||||||||||||||||||
<button id="signalBridge" onClick={signalBridge}> | ||||||||||||||||||||||
Signal Bridge | ||||||||||||||||||||||
</button>, | ||||||||||||||||||||||
document.getElementById('root'), | ||||||||||||||||||||||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ensures the message wont be posted until after the iframe's "onLoad" event fires. This is the way it works with the react bridge.jsx, so we can rely on the consistent ordering of events in the WalletConnection component.