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: nano contract integration with wallet connect #495

Merged
merged 31 commits into from
Aug 16, 2024

Conversation

alexruzenhack
Copy link
Contributor

@alexruzenhack alexruzenhack commented Jun 10, 2024

Acceptance Criteria

  • Add new nano contract modal
    • It should appear when the wallet connect receive an RPC call to create a new nano contract transaction
  • Add new nano contract details screen
    • It should allow the user to change the caller address
    • It should allows the user to decline the transaction
    • It should allows the user to accept the transaction
    • It should show a loading feedback content when transaction information is loading
    • It should show a loading feedback content when transaction is processing after user accepts it
    • It should show a success feedback content when transaction is sent with success
    • It should show a failed feedback content when transaction is not sent with success
    • It should show a feedback content instruction user to register Nano Contract when Nano Contract is not registered
      • It should allows the user to decline the transaction only
    • It should show a loading value on Blueprint Name and Caller on initialize call while background requests are being processed
    • It should show a warning value on Blueprint Name or Caller on initialize call if any request fails
    • It should instruct the user to select an address on Caller if request fails
    • It should allows the user to select an address on Caller if requests fails
    • It should show argument's name if a method call has arguments
    • It should show Position $ for each positional argument as a fallback for argument's name when blueprint info fails to load
    • It should load the details of a not registered token used in actions
      • It should show the symbol for the loaded token details
      • It should show an error message of feedback in case it fails to load some token details
      • It should load a cut of token uid as a fallback for not loading the token details
Loading tx info
wc-nc-loading-tx-info.mp4
Declining tx
wc-nc-declining-tx.mp4
Changing caller address
wc-nc-changing-caller-address.mp4
Accepting tx with failure
wc-nc-accepting-tx-with-failure.mp4
Accepting tx with failure and trying again
wc-nc-accepting-tx-with-failure-try-again.mp4

Accepting tx with success

wc-nc-accepting-tx-with-success.mp4

Handling many types of transactions for Nano Contract registered

fix-initialize-registered-nc.mp4

Handling initialize with requests failure

fix-initialize-loading-to-error.mp4

Handling requests for Nano Contract not registered

fix-initialize-not-registered-nc.mp4

Argument names

Android
iOS

Showing action token symbol for not registered token

Showing UID cut as fallback for not registered token and feedback error message


Warning

It depends on:

  • An API update on hathor-core, related to the nano contract state.
  • Proper implementation of nano contract RPC handler

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.

@alexruzenhack alexruzenhack self-assigned this Jun 10, 2024
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-register-screen branch from f5a626b to 9adfec7 Compare June 13, 2024 14:17
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from ea24255 to 7068889 Compare June 14, 2024 16:05
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-register-screen branch from 0d35063 to 2072e15 Compare June 14, 2024 16:29
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from 7068889 to e7eaa60 Compare June 17, 2024 18:09
Base automatically changed from feat/nano-contract-register-screen to master June 18, 2024 16:28
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from d4645b2 to a110a16 Compare June 20, 2024 16:32
@alexruzenhack alexruzenhack marked this pull request as ready for review June 20, 2024 16:33
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from a110a16 to b2345e7 Compare June 20, 2024 22:09
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from b2345e7 to d79397b Compare July 3, 2024 14:51
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch 2 times, most recently from b236a6f to 3af6727 Compare July 11, 2024 14:39
@alexruzenhack alexruzenhack force-pushed the feat/nano-contract-wallet-connect branch from 9ef9a80 to a35a521 Compare July 16, 2024 22:17
src/actions.js Outdated Show resolved Hide resolved
src/components/HathorFlatList.js Show resolved Hide resolved
src/sagas/wallet.js Outdated Show resolved Hide resolved
src/sagas/wallet.js Outdated Show resolved Hide resolved
locale/pt-br/texts.po Outdated Show resolved Hide resolved
src/constants.js Outdated Show resolved Hide resolved
src/components/FrozenTextValue.js Show resolved Hide resolved
src/components/WalletConnect/theme.js Show resolved Hide resolved
src/components/WalletConnect/theme.js Outdated Show resolved Hide resolved
src/actions.js Show resolved Hide resolved
src/actions.js Show resolved Hide resolved
src/actions.js Outdated Show resolved Hide resolved
src/sagas/wallet.js Outdated Show resolved Hide resolved
Comment on lines 544 to 556
const tokens = {};
let someError = false;
for (const uid of uids) {
try {
const { tokenInfo: { symbol, name } } = yield call([wallet, wallet.getTokenDetails], uid);
const token = { uid, symbol, name };
tokens[uid] = token;
} catch (e) {
log.error(`Fail getting token data for token ${uid}.`, e);
someError = true;
// continue
}
}
Copy link
Contributor

@andreabadesso andreabadesso Aug 13, 2024

Choose a reason for hiding this comment

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

You can use all to make the requests in parallel, also here goes a suggestion for legibility:

Suggested change
const tokens = {};
let someError = false;
for (const uid of uids) {
try {
const { tokenInfo: { symbol, name } } = yield call([wallet, wallet.getTokenDetails], uid);
const token = { uid, symbol, name };
tokens[uid] = token;
} catch (e) {
log.error(`Fail getting token data for token ${uid}.`, e);
someError = true;
// continue
}
}
const tokenDetailsRequests = uids.map((uid) =>
call(function* () {
try {
const { tokenInfo: { symbol, name } } = yield call([wallet, wallet.getTokenDetails], uid);
return [false, { uid, symbol, name }];
} catch (e) {
log.error(`Fail getting token data for token ${uid}.`, e);
return [true, null];
}
})
);
const [someError, tokenDetails] = (yield all(tokenDetailsRequests)).reduce(
(acc, tokenDetailRequest) => {
const [error, tokenObj] = tokenDetailRequest;
const [lastError, tokenDetails] = acc;
return [
lastError || error,
tokenDetail ? {
...tokenDetails,
[tokenDetail.uid]: tokenDetail
} : tokenObj,
];
}, [false, {}]
);

Also, can you move this code to src/sagas/tokens.js and call it here?

Copy link
Contributor Author

@alexruzenhack alexruzenhack Aug 14, 2024

Choose a reason for hiding this comment

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

Oh, thank you! Tbh, I have to think more about it. It involves network calls and may have some hidden pitfalls. I have to test it first. I have had some problems with "parallelism" using promises before. Let me do some tests first. I'm opening this issue #531 to work on it later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you'll have any problems here, even with a huge number of tokens

React-native's native HTTP module will queue them for you and only resolve when the request is done

But I agree that this should be tested, so OK to doing it in another PR

Remarks:
The `marginVertical` and `marginHorizontal` are not being applied. Probably this is a bug in ReactNative.
src/components/FeedbackContent.js Show resolved Hide resolved
Comment on lines 544 to 556
const tokens = {};
let someError = false;
for (const uid of uids) {
try {
const { tokenInfo: { symbol, name } } = yield call([wallet, wallet.getTokenDetails], uid);
const token = { uid, symbol, name };
tokens[uid] = token;
} catch (e) {
log.error(`Fail getting token data for token ${uid}.`, e);
someError = true;
// continue
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think you'll have any problems here, even with a huge number of tokens

React-native's native HTTP module will queue them for you and only resolve when the request is done

But I agree that this should be tested, so OK to doing it in another PR

@alexruzenhack alexruzenhack merged commit 87b5884 into master Aug 16, 2024
2 checks passed
andreabadesso pushed a commit that referenced this pull request Aug 21, 2024
* feat: implement Nano Contract integration with Wallet Connect
* feat: adapt new nc tx to initialize, set result and withdrawal
* feat: make ncAddress react to firstAddress change
* feat: add name to method arguments
* feat: fire sentry error notification for method info not found
* feat: improve title of Arguments component
* feat: retrieve fallback arg entries when blueprint is loading
* feat: add nft check to action token on Actions component
* feat: add blueprint download to the tx loading flag and a redundancy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants