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.

Sign up for GitHub

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

Update balances when TX confirmed or incoming #562

Merged
merged 2 commits into from
Mar 29, 2019
Merged

Update balances when TX confirmed or incoming #562

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b74525a
poll balances when tx happens
brunobar79 Mar 28, 2019
9649a63
Merge branch 'develop' into update-balances-on-txs
Mar 29, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions app/core/TransactionsNotificationManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,15 @@ class TransactionsNotificationManager {
// Clean up
this._removeListeners(transactionMeta.id);

const { TokenBalancesController, AssetsDetectionController, AccountTrackerController } = Engine.context;
// Detect assets and tokens and account balances
// right after a transaction was confirmed
Promise.all([
AccountTrackerController.poll(),
TokenBalancesController.poll(),
AssetsDetectionController.poll()
]);
Copy link
Contributor

@estebanmino estebanmino Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

A thought here.

  • When receiving a tx is going to be a ETH tx for sure, so we could avoid doing TokenBalancesController.poll() and AssetsDetectionController.poll()
  • If we send a tx we now what kind of tx it is ETH or asset ERC20 or ERC721 so we could poll for the specific kind of tx the user is doing, avoiding unnecessary requests (for ex. token balances when sending ETH) both for Infura and the user data

What do you think?

Copy link
Contributor Author

@brunobar79 brunobar79 Mar 29, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

The first point doesn’t apply because it’s only running on submitted txs (Not received) If you look at the received tx block, you’ll ses I’m only updating the eth balance.

Regarding the second point I think that’s fair. Looking at the action key we could decide what to do but I have no idea how to do it so I’ll need your help.


Platform.OS === 'ios' &&
setTimeout(() => {
PushNotification.requestPermissions();
Expand Down Expand Up @@ -236,6 +245,11 @@ class TransactionsNotificationManager {
});
}
}

// Update balance upon detecting
// a new incoming transaction
const { AccountTrackerController } = Engine.context;
AccountTrackerController.poll();
};
}

Expand Down