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

Detect xdai transfers from the bridge #4

Merged
merged 2 commits into from
Apr 14, 2020
Merged

Detect xdai transfers from the bridge #4

merged 2 commits into from
Apr 14, 2020

Conversation

patitonar
Copy link
Contributor

As a continuation to improve the UX of the exchange operations in burner-wallet/burner-wallet-2#21, this PR extends the xdai asset definition to detect when xdai native tokens are minted to the account as a result of exchanging DAI to xDai using the bridge.

With this improvement, the UI will be able to display the incoming tokens to the account in the recent activity list.
exchange

The getTx method was updated to return the data stored in the HistoryEvents if it is a transaction that includes a registered event from the bridge. In that case, it will display that information in the Transaction Receipt details instead of retrieving and showing the actual information of the transaction that will no make much sense because it is a call from a validator to the bridge address with zero value.
receipt

Also, I updated some of the assets types to match the implementation.

this.core.addHistoryEvent({
id: `${event.transactionHash}-${event.logIndex}`,
asset: this.id,
type: 'send',
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want, you could define your own "event type".

Of course, the UI modules (modern-ui) would need to be updated to display them.

@dmihal dmihal merged commit 5072872 into austintgriffith:develop Apr 14, 2020
Copy link

@akolotov akolotov left a comment

Choose a reason for hiding this comment

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

@patitonar thanks for the rapid implementation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants