-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add Oreowallet wallet implementation and facade #91
Conversation
return { networkId: 0 }; | ||
}), | ||
isValidPublicAddress: f.handler.query(({ address }: { address: string }) => { | ||
return isValidPublicAddress(address); |
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.
suggestion: Can we call this directly instead of going through the data facade?
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.
I'd prefer to keep business logic separated from the React code in the facade+wallet layer, and it also mirrors how we do it in the node app (though I know there it's not really an option to call it in the React code). I think the React implementation would basically be a react-query query with this same function anyway?
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.
Ah, is it async? If so, then I agree keeping it here makes more sense
export const walletHandlers = f.facade<WalletHandlers>({ | ||
createAccount: f.handler.mutation( | ||
async ({ name }: { name: string }): Promise<Account> => { | ||
const account = await oreoWallet.createAccount(Network.MAINNET, name); |
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.
Do we want all of these to be hardcoded to mainnet at the moment?
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.
Yep. I still need to build network switching, and our current testnet server doesn't reliably sync up new accounts (Mat's working on getting some fixes in for this)
Adds a wallet and facade implementation using the oreowallet server API.
I'm keeping around the light-server wallet and facades for now for reference, but in the future I'll move/delete them so it's not as confusing which are in use.
This isn't abstracted properly either -- it's pretty close to a point where we could have an IWallet interface that would allow the two wallet implementations to use a common facade, but the transactions from oreowallet are missing a lot of detail (namely the transaction notes). This is changing soon, so it'll be easier at that point.
Also, I think there's a good chance we'll want to cache the API responses, but the extension doesn't do it yet, so I'd rather finish the build process and network switching first.