Skip to content
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: rpc lib integration #500

Merged
merged 101 commits into from
Sep 3, 2024
Merged

feat: rpc lib integration #500

merged 101 commits into from
Sep 3, 2024

Conversation

andreabadesso
Copy link
Contributor

@andreabadesso andreabadesso commented Jun 24, 2024

Screen.Recording.2024-07-12.at.10.07.00.mov

Acceptance Criteria

  • We should enable wallet-connect
  • We shouldn't initialize wallet-connect if the wallet-service is enabled
  • We should be able to handle the htr_signMessageWithAddress and htr_sendNanoContractTx requests
  • We should display modals for user confirmation and send the response back to wallet connect
  • We should crash the entire rootSaga if an error happens in a child saga

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@andreabadesso andreabadesso self-assigned this Jun 24, 2024
@andreabadesso andreabadesso added the enhancement New feature or request label Jun 24, 2024
@andreabadesso andreabadesso changed the base branch from master to feat/nano-contract-wallet-connect June 24, 2024 16:44
@andreabadesso andreabadesso force-pushed the feat/rpc-lib-integration branch 2 times, most recently from c9a1cbc to 66e00e8 Compare June 24, 2024 16:58
@andreabadesso andreabadesso force-pushed the feat/rpc-lib-integration branch from ad99944 to 80cf799 Compare July 3, 2024 14:10
shim.js Outdated
Comment on lines 30 to 45
const { TextEncoder, TextDecoder } = require('text-encoding');

global.TextDecoder = TextDecoder;
global.TextEncoder = TextEncoder;

if (typeof btoa === 'undefined') {
global.btoa = function (str) {
return Buffer.from(str, 'binary').toString('base64');
};
}

if (typeof atob === 'undefined') {
global.atob = function (b64Encoded) {
return Buffer.from(b64Encoded, 'base64').toString('binary');
};
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are needed by wallet-connect

@@ -733,8 +736,8 @@ const App = () => (
>
<NetworkStatusBar />
<RootStack />
<WalletConnectModal />
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we're able to navigate from inside a modal

@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from b2345e7 to d79397b Compare July 3, 2024 14:51
@andreabadesso andreabadesso force-pushed the feat/rpc-lib-integration branch from 4947320 to 57ce051 Compare July 3, 2024 16:18
/>
</ModalBase>
);
};

const WarnDisclaimer = ({ onReadMore }) => (
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sent to components/WalletConnect/WarnDisclaimer to be reused in other modals

@andreabadesso andreabadesso force-pushed the feat/rpc-lib-integration branch 2 times, most recently from 006fdbc to 3a16155 Compare July 11, 2024 12:38
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from b236a6f to 3af6727 Compare July 11, 2024 14:39
const onModalDismiss = useCallback(() => {
dispatch(walletConnectReject());
onDismiss();
}, [onDismiss]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onDismiss comes as a prop, so its reference might change, so we need to pass it as a dep

Comment on lines 43 to 47
useEffect(() => {
if (isRetrying) {
navigatesToNewNanoContractScreen();
}
}, [isRetrying, navigation]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is to bypass the initial modal when the user is retrying

Without this "redirect", the user would have to press review in this screen again, after pressing "retry":

image

Comment on lines 163 to 165
if (this.props.route.params.dispatchResponse) {
pinScreenResult(pin);
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could have passed a callback in the route params, but I don't think that's ideal as it yields a warning, I created this new param dispatchResponse as a new mechanism for getting the result in the sagas

We should refactor this in a future PR to stop passing callbacks as params

@andreabadesso andreabadesso force-pushed the feat/rpc-lib-integration branch from 5208d8f to 6a33a5f Compare July 12, 2024 12:59
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from 9ef9a80 to a35a521 Compare July 16, 2024 22:17
@alexruzenhack alexruzenhack force-pushed the feat/rpc-lib-integration branch from 72be982 to 56059a2 Compare July 18, 2024 14:48
Copy link
Contributor

@alexruzenhack alexruzenhack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed the error handling on saga initialization and I'm letting at your discretion to create or not a simple implementation to halt a saga restart, or defer it to another PR.

@andreabadesso andreabadesso force-pushed the feat/rpc-lib-integration branch from b23f09c to 2a6ebf7 Compare August 31, 2024 21:09
@andreabadesso andreabadesso merged commit c306c25 into master Sep 3, 2024
1 check passed
@andreabadesso andreabadesso deleted the feat/rpc-lib-integration branch September 3, 2024 16:01
@tuliomir tuliomir mentioned this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants