Update currency string representation #115

diehuxx · 2023-12-14T22:56:29Z

Update currency amount representation and validation based on this spec change: TBD54566975/tbdex#206

Changes based on the spec

Update the protocol spec, test-vectors, and examples to represent currencies as decided in issue #199 in this comment here.

Strings

Containing major unit amounts

Decimals using period . as the decimal separator regardless of currency.
Currency representation tbdex#199 (comment)

The updated fields taken from this comment here.

Old New

offering.payinCurrency.minSubunits minAmount

offering.payinCurrency.maxSubunits maxAmount

offering.payoutCurrency.minSubunits minAmount

offering.payoutCurrency.maxSubunits maxAmount

offering.paymentMethods[*].feeSubunits fee

rfq.payinSubunits payinAmount

quote.payin.amountSubunits amount

quote.payin.feeSubunits fee

quote.payout.amountSubunits amount

quote.payout.feeSubunits fee

Notes

I added an await Message.validate(<message-object> to each #create method. Since the updated test-vectors in the spec change PR include regex for decimal strings, we can use Message.validate() to check that currency strings are in fact valid.

changeset-bot · 2023-12-14T22:56:33Z

⚠️ No Changeset found

Latest commit: 6bfbd09

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

kirahsapong

nice!

kirahsapong · 2023-12-19T22:30:40Z

packages/protocol/src/dev-tools.ts

@@ -202,8 +202,8 @@ export class DevTools {
          btcAddress: '1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa'
        }
      },
-      payinSubunits : '20000',
-      claims        : [signedCredential]
+      payinAmount : '20000',


Suggested change

payinAmount : '20000',

payinAmount : '200.00',

kirahsapong · 2023-12-19T22:33:34Z

packages/protocol/src/types.ts

@@ -226,9 +226,9 @@ export type QuoteDetails = {
  /** ISO 3166 currency code string */
  currencyCode: string
  /** The amount of currency expressed in the smallest respective unit */


Suggested change

/** The amount of currency expressed in the smallest respective unit */

/** The amount of currency to two decimal places expressed as a string (ie. '1000.00')*/

kirahsapong · 2023-12-19T22:35:01Z

packages/protocol/tests/rfq.spec.ts

@@ -21,8 +21,8 @@ const rfqData: RfqData = {
      btcAddress: '1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa'
    }
  },
-  payinSubunits : '20000',
-  claims        : ['']
+  payinAmount : '20000',


Suggested change

payinAmount : '20000',

payinAmount : '200.00',

kirahsapong · 2023-12-19T22:35:11Z

packages/protocol/tests/rfq.spec.ts

@@ -190,8 +190,8 @@ describe('Rfq', () => {
              btcAddress: '1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa'
            }
          },
-          payinSubunits : '20000',
-          claims        : [signedCredential]
+          payinAmount : '20000',


Suggested change

payinAmount : '20000',

payinAmount : '200.00',

kirahsapong · 2023-12-19T22:35:18Z

packages/protocol/tests/rfq.spec.ts

@@ -229,8 +229,8 @@ describe('Rfq', () => {
              btcAddress: '1A1zP1eP5QGefi2DMPTfTL5SLmv7DivfNa'
            }
          },
-          payinSubunits : '20000',
-          claims        : [signedCredential]
+          payinAmount : '20000',


Suggested change

payinAmount : '20000',

payinAmount : '200.00',

phoebe-lew · 2023-12-20T06:56:13Z

packages/http-server/src/fakes.ts

    },
-    payoutUnitsPerPayinUnit : '0.000038',
-    payinMethods            : [{
+    rate         : '0.000038',


Why the rename to rate? We have had issues knowing which direction the rate is hence the explicitness

It was suggested in the issue and I liked it. But I'll revert if it's controversial.

phoebe-lew · 2023-12-20T06:59:51Z

packages/protocol/src/message-kinds/close.ts

@@ -31,7 +31,9 @@ export class Close extends Message<'close'> {
    }

    const message = { metadata, data: opts.data }
-    return new Close(message)
+    const close = new Close(message)
+    await Message.validate(close)


Message.validate is not async so I don't think we need to await this

Decided to remove Message.validate entirely since it's requires signature to be present even though the message is not signed at this point.

phoebe-lew · 2023-12-20T07:00:16Z

packages/protocol/src/resource-kinds/offering.ts

@@ -31,7 +31,9 @@ export class Offering extends Resource<'offering'> {
    }

    const message = { metadata, data: opts.data }
-    return new Offering(message)
+    const offering = new Offering(message)
+    await Resource.validate(offering)


Resource.validate is not async

phoebe-lew · 2023-12-20T07:00:52Z

packages/protocol/src/message-kinds/order-status.ts

@@ -32,7 +33,9 @@ export class OrderStatus extends Message<'orderstatus'> {
    }

    const message = { metadata, data: opts.data }
-    return new OrderStatus(message)
+    const orderStatus = new OrderStatus(message)
+    await Message.validate(order)


ditto not async

phoebe-lew · 2023-12-20T07:01:01Z

packages/protocol/src/message-kinds/order.ts

@@ -31,6 +31,8 @@ export class Order extends Message<'order'> {
    }

    const message = { metadata, data: {} }
-    return new Order(message)
+    const order = new Order(message)
+    await Message.validate(order)


ditto not async

phoebe-lew · 2023-12-20T07:01:07Z

packages/protocol/src/message-kinds/quote.ts

@@ -32,7 +32,9 @@ export class Quote extends Message<'quote'> {
    }

    const message = { metadata, data: opts.data }
-    return new Quote(message)
+    const quote = new Quote(message)
+    await Message.validate(quote)


ditto not async

phoebe-lew · 2023-12-20T07:01:14Z

packages/protocol/src/message-kinds/rfq.ts

@@ -44,7 +44,9 @@ export class Rfq extends Message<'rfq'> {
    // TODO: hash `data.payoutMethod.paymentDetails` and set `private`

    const message = { metadata, data: opts.data }
-    return new Rfq(message)
+    const rfq = new Rfq(message)
+    await Message.validate(rfq)


ditto not async

diehuxx · 2024-01-14T00:11:06Z

Closing this PR because I forgot about (woops) and then reimplemented this change but more thoroughly in #134.

diehuxx added 2 commits December 14, 2023 14:42

Update currency string representation

a40c636

Validate structure of messages and resources during creation

1ef9114

diehuxx requested review from michaelneale, mistermoe, jiyoontbd, phoebe-lew and KendallWeihe as code owners December 14, 2023 22:56

diehuxx mentioned this pull request Dec 14, 2023

#199 Update currency representation TBD54566975/tbdex#206

Merged

diehuxx added 2 commits December 14, 2023 15:55

Lint

80975b6

await

84dae7c

kirahsapong reviewed Dec 19, 2023

View reviewed changes

phoebe-lew reviewed Dec 20, 2023

View reviewed changes

diehuxx added 2 commits January 5, 2024 10:35

PR comments

3699b57

One more

6bfbd09

diehuxx closed this Jan 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update currency string representation #115

Update currency string representation #115

diehuxx commented Dec 14, 2023 •

edited

Loading

changeset-bot bot commented Dec 14, 2023 •

edited

Loading

kirahsapong left a comment

kirahsapong Dec 19, 2023

kirahsapong Dec 19, 2023

kirahsapong Dec 19, 2023

kirahsapong Dec 19, 2023

kirahsapong Dec 19, 2023

phoebe-lew Dec 20, 2023

diehuxx Jan 5, 2024

phoebe-lew Dec 20, 2023

diehuxx Jan 5, 2024

phoebe-lew Dec 20, 2023

phoebe-lew Dec 20, 2023

phoebe-lew Dec 20, 2023

phoebe-lew Dec 20, 2023

phoebe-lew Dec 20, 2023

diehuxx commented Jan 14, 2024

Old	New
`offering.payinCurrency.minSubunits`	`minAmount`
`offering.payinCurrency.maxSubunits`	`maxAmount`
`offering.payoutCurrency.minSubunits`	`minAmount`
`offering.payoutCurrency.maxSubunits`	`maxAmount`
`offering.paymentMethods[*].feeSubunits`	`fee`
`rfq.payinSubunits`	`payinAmount`
`quote.payin.amountSubunits`	`amount`
`quote.payin.feeSubunits`	`fee`
`quote.payout.amountSubunits`	`amount`
`quote.payout.feeSubunits`	`fee`

	/** The amount of currency expressed in the smallest respective unit */
	/** The amount of currency to two decimal places expressed as a string (ie. '1000.00')*/

Update currency string representation #115

Update currency string representation #115

Conversation

diehuxx commented Dec 14, 2023 • edited Loading

Changes based on the spec

Notes

changeset-bot bot commented Dec 14, 2023 • edited Loading

⚠️ No Changeset found

kirahsapong left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diehuxx commented Jan 14, 2024

diehuxx commented Dec 14, 2023 •

edited

Loading

changeset-bot bot commented Dec 14, 2023 •

edited

Loading