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

Jump to bottom

Add wallet support for decimal currencies and some examples #1984

Merged
merged 9 commits into from
Nov 5, 2020
Merged

Add wallet support for decimal currencies and some examples #1984

merged 9 commits into from
Nov 5, 2020

Conversation

michaelfig
Copy link
Member

@michaelfig michaelfig commented Nov 5, 2020 •
edited
Loading

Closes #1978, closes #1977

We don't actually have a path in front of the issuer, but we pretend by using 'Testnet.$USD' and 'Testnet.$LINK'.

The support for decimals in the wallet is genuine, though.

Screen Shot 2020-11-04 at 7 18 44 PM

@michaelfig michaelfig added ERTP package: ERTP cosmic-swingset package: cosmic-swingset wallet labels Nov 5, 2020
@michaelfig michaelfig added this to the Chainlink Hackathon milestone Nov 5, 2020
@michaelfig michaelfig self-assigned this Nov 5, 2020
katelynsills
katelynsills suggested changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

@katelynsills katelynsills left a comment

Choose a reason for hiding this comment

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

I think we should not be sending display-appropriate values to the wallet and parsing there. I think the UI should be responsible to both displaying and reversing the display changes such the wallet only sees nat values for fungible tokens.

packages/dapp-svelte-wallet/ui/src/Amount.svelte Outdated Show resolved Hide resolved
packages/dapp-svelte-wallet/api/src/lib-wallet.js Outdated Show resolved Hide resolved
FUDCo
FUDCo approved these changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

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

Seems mostly OK, save my one comment about JSON5, which is less a comment about this PR and more flagging that we have a broader issue we should probably establish a policy on.

packages/dapp-svelte-wallet/api/src/lib-wallet.js Outdated Show resolved Hide resolved
katelynsills
katelynsills approved these changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

@katelynsills katelynsills left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for making those changes!

@michaelfig michaelfig merged commit 97c10d0 into master Nov 5, 2020
@michaelfig michaelfig deleted the mfig/more-currencies branch November 5, 2020 23:42
This was referenced Nov 28, 2020
Open
Open
This was referenced Oct 22, 2022
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@erights erights erights left review comments

@katelynsills katelynsills katelynsills approved these changes

@FUDCo FUDCo FUDCo approved these changes

@kriskowal kriskowal Awaiting requested review from kriskowal

Assignees

@michaelfig michaelfig

Labels
cosmic-swingset package: cosmic-swingset ERTP package: ERTP wallet
Projects
None yet
Milestone
Chainlink Hackathon
Development

Successfully merging this pull request may close these issues.

Denominations on the sim-chain testnet Decimal support in Wallet UI
4 participants
@michaelfig @erights @katelynsills @FUDCo