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

Add support for generic CIP30 wallets by name #1528

Closed

Conversation

danielfarrelly
Copy link
Contributor

Pre-review checklist

  • All code has been formatted using our config (make format)
  • Any new API features or modification of existing behavior is covered as defined in our test plan
  • The changelog has been updated under the ## Unreleased header, using the appropriate sub-headings (### Added, ### Removed, ### Fixed), and the links to the appropriate issues/PRs have been included

@danielfarrelly
Copy link
Contributor Author

Recreated from #1524.

@klntsky klntsky self-requested a review August 17, 2023 15:54
Nami only provides `getCollateral` under the experimental API
*/
(typeof conn.getCollateral === "function"
? conn.getCollateral("5000000")
Copy link
Member

Choose a reason for hiding this comment

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

This string value is probably incorrect. In the spec, it must be cbor-string of Coin type, not numeric literal.

@klntsky
Copy link
Member

klntsky commented Aug 18, 2023

We should pass the amount parameter from purescript. If we hardcode it, it must be hardcoded on PS side and then processed into cbor as usual

@danielfarrelly
Copy link
Contributor Author

We should pass the amount parameter from purescript. If we hardcode it, it must be hardcoded on PS side and then processed into cbor as usual

Looks like this could actually be reverted to passing no arguments with the latest release of Yoroi:
Emurgo/yoroi-frontend@2abc5fd

@klntsky
Copy link
Member

klntsky commented Sep 17, 2023

@danielfarrelly

No, we must require it, because this is what CIP-30 tells us to do:

https://cips.cardano.org/cips/cip30/#apigetcollateralparamsamountcborcoinpromisetransactionunspentoutputnull

The function takes a required object with parameters. With a single required parameter for now: amount. (NOTE: some wallets may be ignoring the amount parameter, in which case it might be possible to call the function without it, but this behavior is not recommended!). Reasons why the amount parameter is required: [...]

The current behavior is a bug.

@klntsky klntsky self-assigned this Sep 22, 2023
@klntsky
Copy link
Member

klntsky commented Sep 22, 2023

I'm taking this PR over

@klntsky
Copy link
Member

klntsky commented Sep 25, 2023

Re-created here: #1536

@klntsky klntsky closed this Sep 25, 2023
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.

2 participants