CIP-0030: getCollateral({amount}) argument type

[itsfarseen]

CIPs/CIP-0030/README.md

Line 282 in 5498717

#### `api.getCollateral(params: { amount: cbor<Coin> }): Promise<TransactionUnspentOutput[] | null>`

api.getCollateral(params: { amount: cbor<Coin> }): Promise<TransactionUnspentOutput[] | null>

The type signature says amount: cbor<Coin>.

CIPs/CIP-0030/README.md

Line 297 in 5498717

The `amount` parameter is required, specified as a `string` (BigNumber) or a `number`, and the maximum allowed value must be agreed to be something like 5 ADA. Not limiting the maximum possible value might force the wallet to attempt to purify an unreasonable amount of ADA just because the dapp is doing something weird. Since by protocol the required collateral amount is always a percentage of the transaction fee, it seems that the 5 ADA limit should be enough for the foreseeable future.

The amount parameter is required, specified as a string (BigNumber) or a number, and the maximum allowed value must be agreed to be something like 5 ADA. Not limiting the maximum possible value might force the wallet to attempt to purify an unreasonable amount of ADA just because the dapp is doing something weird. Since by protocol the required collateral amount is always a percentage of the transaction fee, it seems that the 5 ADA limit should be enough for the foreseeable future.

Here it says amount can be a BigNumber converted to string or a JS number.

Which is the correct version?

[rphair]

good catch @itsfarseen ... I'd imagine one of the two is more commonly done in practice, and maybe it should be set to that; @Ryun1 @Crypto2099 any ideas?

[itsfarseen]

@rphair I looked into common implementations by doing a Github code search. All implementations I found ignore the amount parameter.
I think this used to be the behavior of certain wallets, on which the function described in this CIP is based.

[itsfarseen]

cc @vsubhuman, author of #208

[rphair]

@itsfarseen yes, that makes more sense now; in my non-dev experience I recall all wallets seem to use the "5 ada" convention. Still I'm glad we've got this on record, and if this parameter is ignored by common trade practice then I believe this should be referenced in CIP-0030... or if not then as a pending issue in @Ryun1's candidate CPS-0010 (#619).

[klntsky]

if this parameter is ignored by common trade practice then I believe this should be referenced in CIP-0030...

Bug reports should better be created in the appropriate repositories and not in the CIPs

[klntsky]

@itsfarseen could you please list the implementations that ignore the amount with links to the code

[rphair]

@klntsky #636 (comment): Bug reports should better be created in the appropriate repositories

Of course that's true about the wallet bug reports, but here we also have a CIP contradicting itself between the two source lines above, just a couple paragraphs apart. If I've mis-read that there's a contradiction there, please let me know how those 2 reported statements about the data type could both be true. Otherwise we need to prepare some kind of fix for the CIP text itself.

[itsfarseen]

cardano-js-sdk/dapp-connector
Seems to correctly specify the types:
https://github.com/input-output-hk/cardano-js-sdk/blob/6c96c835f2578492c96e137ac3d9d5e33b9c390b/packages/dapp-connector/src/WalletApi/types.ts#L67

Yoroi
Seems to have an amount parameter. But it's hard to infer the type from the code:
https://github.com/Emurgo/yoroi-frontend/blob/3f5752480eeb3df62f3a3446b8d0f46f3e3bfa19/packages/yoroi-connector/src/cardanoApiInject.js#L170

This PR says Emurgo/yoroi-frontend#3279 (comment):

Both .getCollateral and the deprecated .getCollateralUtxos functions can now be called with one of these parameters:

A number as a number
A number as a string
An object { amount: string | number } (proper CIP30)
A CBOR encoding of type Value
No parameter (in which case the default max allowed value of 5 ADA will be used)

Note: CIP-0030 function type says getCollateral(param: {amount: CBOR}) which is not included in the above list of supported values.
The wallets which ignore the amount value may continue to work, when {amount: <cbor hex string>} is passed as param.
Yoroi may fail further down the line trying to interpret it as {amount: <number as string>}.

Examples where the amount is ignored

1. Lucid is a library which interacts with CIP30 wallets.
   (Although they have a typedef for the function, they don't seem to be calling it)
   https://github.com/spacebudz/lucid/blob/457c156e74ef49ce35823aa2bca91b3bbc585221/src/types/global.ts#L16
2. Nami seems to ignore the amount parameter and hardcode the limit as 5 ADA.
   https://github.com/input-output-hk/nami/blob/551985f0cc163173c9de231c956b502d0f1c93b1/src/api/extension/index.js#L372
3. Indigo Dexter
   https://github.com/IndigoProtocol/dexter/blob/220473a0afc72316fc1521aab357a5d1e6cd0fb2/src/types.ts#L137
4. dcSpark adalib
   https://github.com/dcSpark/adalib/blob/c0f2530bfcdc13927642a9ece168933beddedf7e/src/actions/accounts.ts#L69
   https://github.com/dcSpark/adalib/blob/c0f2530bfcdc13927642a9ece168933beddedf7e/src/types/CardanoInjected.ts#L82

[rphair]

I'd hoped to get a quick answer at the CIP meeting today where this was briefly discussed.

The best we can do for now, since we didn't have anyone stepping forward with a conclusive answer about how to solve the contradiction in the CIP, was that @Ryun1 or @Crypto2099 will, via the CIP Editors' Discord, open or lead a wallet dev discussion about (generally?) how collateral is characterised and (specifically?) which data type would best be implied by current usage.

@itsfarseen you would be welcome in that discussion so please post if you need a reference or Discord invite there.

[itsfarseen]

I propose to use CBOR and disallow any other types, so that:

• It's consistent with the rest of the API.
• CIP30 implementors won't have to do runtime type detection, which can be error prone and inconsistent across implementations.

---

That said, I do feel that CIP-30 functions accepting and returning CBOR strings for every single argument is not ideal.

We should have standardized JS types (using JSDoc or .d.ts files), similar to the CDDL types. All JS code should be able to exchange data using these types and should only need to convert to CBOR at the boundary of interaction with cardano nodes/RPCs.

[itsfarseen]

@rphair I'd love to join the discussion. My Discord username is itsfarseen.

[rphair]

@itsfarseen & others: a Discord invite to the CIP discussion server is: https://discord.gg/tnQAjZYcTr - if there's a thread for this issue created there as per #636 (comment) we'll link to it here.

[rphair]

@itsfarseen @Ryun1 @Crypto2099 this issue was given some attention in today's CIP meeting (after getting bumped off last agenda or two due to time constraints) and has been escalated to the Wallets Working Group meeting in about 2 days' time for concentrated visibility & resolving it there.