regenerated test vector to add a valid DID for to field in `Message.metadata.from|to` #247

jiyoonie9 · 2024-02-22T23:24:10Z

No description provided.

jiyoonie9 · 2024-02-23T00:04:35Z

hosted/test-vectors/protocol/vectors/parse-close.json

@@ -1,19 +1,19 @@
 {
  "description": "Close parses from string",
-  "input": "{\"metadata\":{\"from\":\"did:dht:abecscpi377h647nj5j8oz6yq544i8bzdze83mxsghskahq35ddy\",\"to\":\"did:ex:alice\",\"exchangeId\":\"abcd123\",\"kind\":\"close\",\"id\":\"close_01hpsc6aqten79yhw1gg25prmj\",\"createdAt\":\"2024-02-16T16:32:31.482Z\"},\"data\":{\"reason\":\"The reason for closing the exchange\"},\"signature\":\"eyJhbGciOiJFZERTQSIsImtpZCI6ImRpZDpkaHQ6YWJlY3NjcGkzNzdoNjQ3bmo1ajhvejZ5cTU0NGk4YnpkemU4M214c2doc2thaHEzNWRkeSMwIn0..LfYrchKuy0KtdRl-HRgLDSwRvzVAz6mApuQdJU5prOchLMRJkzcjROWxn5JbzkwVajH6VN4Xmhv6UcOfmZmoCA\"}",
+  "input": "{\"metadata\":{\"from\":\"did:dht:5feinba343k9fhy4zgdcp64bm6y7si39ru1pfjmrbxw9tkggbeuo\",\"to\":\"did:dht:t13oezyg87588ja11zoox6swtt7a8gq4zbmc6su5xbjy8xgdb46o\",\"exchangeId\":\"rfq_01hq9j1a70f3zrrh11n3q9x3jk\",\"kind\":\"close\",\"id\":\"close_01hq9j1a71fgh9qszsx06mefyn\",\"createdAt\":\"2024-02-22T23:22:29.473Z\"},\"data\":{\"reason\":\"The reason for closing the exchange\"},\"signature\":\"eyJhbGciOiJFZERTQSIsImtpZCI6ImRpZDpkaHQ6NWZlaW5iYTM0M2s5Zmh5NHpnZGNwNjRibTZ5N3NpMzlydTFwZmptcmJ4dzl0a2dnYmV1byMwIn0..Nnn8v5RZIgkA8rc9TC9WgKqdx1xe9F8197vhLqmlMQxv-U62jbORV5vNJXdzPlOo6PfgK84OdAAx1cuhsvcZCw\"}",


all these were regenerated because Message.metadata.to field was did:ex:alice and that fails our schema validation - it must be a real DID

…bdex into test-vector-update

KendallWeihe · 2024-02-23T13:10:18Z

Could you add more description as to what made the pre-existing DIDs invalid? (or maybe point to a ticket which describes the issue)

I'm cool with the CODEOWNERs change, though it's worthy of a discussion (changes here being so upstream of everything else makes changes here have leverage, resulting in large downstream implications; my point being, if there ever were a need to create a constitution of governance then it should be had here -- "we're not there yet" is a valid response such that we can iterate quicker right now). Which, there may have been a discussion for which I wasn't privy to, but if not, could we make the CODEOWNERs a dedicated PR and/or ticket on the backlog?

jiyoonie9 · 2024-02-23T13:39:22Z

Could you add more description as to what made the pre-existing DIDs invalid? (or maybe point to a ticket which describes the issue)

yes, can do! so i found this while getting my tbdex-js pr up to speed with main, and found that tbdex had some schema changes that enforced Message.metadata.from/to to be a real DID. the test vector was not updated with this change, and some of the test vectors had the value for those fields set to an invalid DID like did:ex:alice or did:ex:pfi. tests in tbdex-js and tbdex-kt that used the test vectors were failing because of this

I'm cool with the CODEOWNERs change, though it's worthy of a discussion (changes here being so upstream of everything else makes changes here have leverage, resulting in large downstream implications; my point being, if there ever were a need to create a constitution of governance then it should be had here -- "we're not there yet" is a valid response such that we can iterate quicker right now). Which, there may have been a discussion for which I wasn't privy to, but if not, could we make the CODEOWNERs a dedicated PR and/or ticket on the backlog?

happy to move the codeowners change out to a separate PR!
edit: pr here - #248

…o test-vector-update

jiyoonie9 · 2024-02-23T13:54:18Z

hosted/json-schemas/message.schema.json

@@ -11,7 +11,7 @@
          "description": "The sender's DID"
        },
        "to": {
-          "$ref": "https://tbdex.dev/definitions.json#/definitions/did",
+          "$ref": "definitions.json#/definitions/did",


changed to be consistent with how we use $ref elsewhere https://github.com/TBD54566975/tbdex/pull/247/files#diff-781d77c475148a35130d81220b3a6d100357fc53576806be6dda9a528f1ae283L10

KendallWeihe · 2024-02-23T13:59:37Z

the test vectors had the value for those fields set to an invalid DID like did:ex:alice or did:ex:pfi

Got it, thanks! A good amount of these seem like did:dht-to-did:jwk changes. I would imagine we want did:dht in some of our test vectors, albeit maybe not currently? Or maybe it's beside the point and either or are fine and the changes here were just a byproduct of you generating new test vectors. Just raising awareness cc @phoebe-lew

jiyoonie9 · 2024-02-23T16:57:57Z

the test vectors had the value for those fields set to an invalid DID like did:ex:alice or did:ex:pfi

Got it, thanks! A good amount of these seem like did:dht-to-did:jwk changes. I would imagine we want did:dht in some of our test vectors, albeit maybe not currently? Or maybe it's beside the point and either or are fine and the changes here were just a byproduct of you generating new test vectors. Just raising awareness cc @phoebe-lew

yes, so i actually did initially opt to use did:dht, but found an issue while using did:dht:... for DIDs in test vectors and then ran tests that failed in tbdex-kt. decentralized-identity/web5-kt#252

i should probably write an issue to revert the DIDs back to did:dht once that issue is resolved. wrote issue here: #249

mistermoe · 2024-02-24T01:00:23Z

I thiiink we may want to keep the PFI DIDs as did:dht beeecause PFIs won't ever be able to use did:jwk beeecauuse they don't have support for service endpoints.

Thoughts?

jiyoonie9 · 2024-02-24T02:44:07Z

So would that mean we have to first address the did:dht issue i found while running test vector tests in tbdex-kt? I only used did:jwk to unblock myself so all the tests would pass with the latest test vectors.

…

On Fri, Feb 23, 2024 at 10:01 PM Moe Jangda ***@***.***> wrote: I thiiink we may want to keep the PFI DIDs as did:dht beeecause PFIs won't ever be able to use did:jwk beeecauuse they don't have support for service endpoints. Thoughts? — Reply to this email directly, view it on GitHub <#247 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/A4IFEFYPB5AHMOY3CLPAMDLYVE3WFAVCNFSM6AAAAABDV2FWY6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRSGE4DQMZYGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

mistermoe · 2024-02-25T06:07:10Z

@jiyoontbd just fixed the issue you found earlier!

Related issue: decentralized-identity/web5-kt#252
PR with fix: decentralized-identity/web5-kt#251
PR with updates to test vector generation: TBD54566975/tbdex-js#185

diehuxx · 2024-02-26T18:53:24Z

hosted/test-vectors/protocol/vectors/parse-offering.json

  "output": {
    "metadata": {
-      "from": "did:dht:abu39bcp7peyxkff8yz85kds4k4gpoizigkdcyqx3ozbyyiua6po",
+      "from": "did:dht:1uh6jhzqsreubaunfcrmaqux5nq66hxm3fmj9wpt4w1zmpn6mzuy",


Since parse-offering.json was already using did:dht, will this make a difference?

no, it would not, but all test vectors get re-generated with the generateXYZ() methods, so that's why these change even though they were did:dhts before

regenerated test vector to add a valid DID for to field in parse-close

3b3dca2

jiyoonie9 mentioned this pull request Feb 22, 2024

During request token verification, check that JWTHeader.kid includes the same DID as JWTClaims.iss TBD54566975/tbdex-js#176

Merged

jiyoonie9 marked this pull request as ready for review February 23, 2024 00:03

jiyoonie9 requested review from michaelneale, mistermoe, phoebe-lew and KendallWeihe as code owners February 23, 2024 00:03

jiyoonie9 commented Feb 23, 2024

View reviewed changes

jiyoonie9 added 4 commits February 22, 2024 21:38

adding more ppl to codeowners

9c4e970

regen test vectors

755bd7c

Merge branch 'test-vector-update' of https://github.com/TBD54566975/t…

243119c

…bdex into test-vector-update

trying out didjwk instead

44f8db7

jiyoonie9 mentioned this pull request Feb 23, 2024

pulling in latest test vector update from tbdex TBD54566975/tbdex-kt#181

Merged

jiyoonie9 added 4 commits February 23, 2024 10:40

backing out codeowners change, to be done in separate pr

9392984

Merge branch 'test-vector-update' of github.com:TBD54566975/tbdex int…

0d01c55

…o test-vector-update

newline

897c27e

removing tbdex.dev host name for did definition references

27eed58

jiyoonie9 changed the title ~~regenerated test vector to add a valid DID for to field in parse-close~~ regenerated test vector to add a valid DID for to field in Message.metadata.from|to Feb 23, 2024

jiyoonie9 commented Feb 23, 2024

View reviewed changes

jiyoonie9 mentioned this pull request Feb 23, 2024

Add protocol field to protocol spec #244

Merged

diehuxx approved these changes Feb 23, 2024

View reviewed changes

kirahsapong approved these changes Feb 23, 2024

View reviewed changes

try did:dht dids

ea435d7

mistermoe mentioned this pull request Feb 25, 2024

Test vector generation updates TBD54566975/tbdex-js#185

Merged

diehuxx reviewed Feb 26, 2024

View reviewed changes

diehuxx approved these changes Feb 26, 2024

View reviewed changes

KendallWeihe approved these changes Feb 26, 2024

View reviewed changes

jiyoonie9 merged commit 50cbc23 into main Feb 26, 2024

jiyoonie9 deleted the test-vector-update branch February 26, 2024 19:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

regenerated test vector to add a valid DID for to field in `Message.metadata.from|to` #247

regenerated test vector to add a valid DID for to field in `Message.metadata.from|to` #247

jiyoonie9 commented Feb 22, 2024

jiyoonie9 Feb 23, 2024

KendallWeihe commented Feb 23, 2024

jiyoonie9 commented Feb 23, 2024 •

edited

Loading

jiyoonie9 Feb 23, 2024

KendallWeihe commented Feb 23, 2024

jiyoonie9 commented Feb 23, 2024 •

edited

Loading

mistermoe commented Feb 24, 2024 •

edited

Loading

jiyoonie9 commented Feb 24, 2024 via email

mistermoe commented Feb 25, 2024 •

edited

Loading

diehuxx Feb 26, 2024

jiyoonie9 Feb 26, 2024

regenerated test vector to add a valid DID for to field in Message.metadata.from|to #247

regenerated test vector to add a valid DID for to field in Message.metadata.from|to #247

Conversation

jiyoonie9 commented Feb 22, 2024

jiyoonie9 Feb 23, 2024

Choose a reason for hiding this comment

KendallWeihe commented Feb 23, 2024

jiyoonie9 commented Feb 23, 2024 • edited Loading

jiyoonie9 Feb 23, 2024

Choose a reason for hiding this comment

KendallWeihe commented Feb 23, 2024

jiyoonie9 commented Feb 23, 2024 • edited Loading

mistermoe commented Feb 24, 2024 • edited Loading

jiyoonie9 commented Feb 24, 2024 via email

mistermoe commented Feb 25, 2024 • edited Loading

diehuxx Feb 26, 2024

Choose a reason for hiding this comment

jiyoonie9 Feb 26, 2024

Choose a reason for hiding this comment

regenerated test vector to add a valid DID for to field in `Message.metadata.from|to` #247

regenerated test vector to add a valid DID for to field in `Message.metadata.from|to` #247

jiyoonie9 commented Feb 23, 2024 •

edited

Loading

jiyoonie9 commented Feb 23, 2024 •

edited

Loading

mistermoe commented Feb 24, 2024 •

edited

Loading

mistermoe commented Feb 25, 2024 •

edited

Loading