Skip to content

Conversation

@aljones15
Copy link
Contributor

@aljones15 aljones15 commented Nov 17, 2024

Adds 3 proofValue related tests:

  1. "If the proofValue string does not start with u, indicating that it is a multibase-base64url-no-pad-encoded value, an error MUST be raised and SHOULD convey an error type of PROOF_VERIFICATION_ERROR"
  "proof": {
    "type": "DataIntegrityProof",
    "created": "2024-11-17T15:51:32Z",
    "verificationMethod": "did:key:zDnaepBuvsQ8cpsWrVKw8fbpGpvPeNSjVPTWoq6cRqaYzBKVP#zDnaepBuvsQ8cpsWrVKw8fbpGpvPeNSjVPTWoq6cRqaYzBKVP",
    "cryptosuite": "ecdsa-sd-2023",
    "proofPurpose": "assertionMethod",
    "proofValue": "2V0BhVhANk3uoC2QPhe9knhEUtqpGS9cdbp1oRKf3SZ9v1iUO-UKS-hFqgacRYOrCSh8duVDK_zY5N5Hndkq8l7VRVAMNFgjgCQDR-DKGWfNv3Kq9TKO1wKXsgvmVlsjqhTWw4U8N-zL23yHWEAjupTpyzvis37wkzIwiXKuizfcF9VvpZfMx7uF2tVW3Al-BbTXQy_HO-EOM3WdH_sZO7K0CP_8fgGMk3UwsmrzWEDBizvJxii_nXROPhqvulJp1e27ZmixVmPUm9ZMT-zl-WAoocGzZnLfwdP_ujRfZVSs_CSJRnz87cqKtgRxG0HqWEBTp4qhxcXJbNVup9iCZAlHLt5oKEYNDEn2KWFMeUqBtwFAWjbVDOQQbPiiZ8XN6Ko8sD0MsSmQ28YkyQD9sir5WEA00uDbdz-gaeeQwPMm4gO-eLWSoIx9LRGLK8F5QEJGa4amwfn8eJ178DUCpr1BRZoGiJAkVR8AzE1Xk8vuXF-JWEAgknRMffrzujB2IW79_1O0SnQf9NyOzPpsws5dbhD8l5PsrzQI6HFaZrhSNayha0ppCw50UdtMo2zSRXWYoVMRWECaWckwEmY6BgddbWySxeYgUwHB3p72kxQ7Eg-9Idd5Z9fEcj8tFwCFDjZJJdXKhBSM71J8dAf9HVZo12Wm51_CWECmmVvtaKWgUlvRoPN6G6wfV25e4fPeo4d8l1ApSllxZ19Ls7-uCRl2jE28NbrCvzmPfg3mLbQBRhM3fabvdcbhoQBYIKzZT5KJrjiCO1Dhv0Ww6rqGS8K_yY56UpQc9j12-oHohAECBAU"
  }
  1. The base proof assertion: "If the decodedProofValue does not start with the ECDSA-SD base proof header bytes 0xd9, 0x5d, and 0x00, an error MUST be raised and SHOULD convey an error type of PROOF_VERIFICATION_ERROR." Is tested by examing the base proof from the issuer. As base proofs are not designed to pass verification the statement from the spec might be incorrect.

  2. "If the decodedProofValue does not start with the ECDSA-SD disclosure proof header bytes 0xd9, 0x5d, and 0x01, an error MUST be raised and SHOULD convey an error type of PROOF_VERIFICATION_ERROR."

The invalid disclosure proof header bytes are [0xA1, 0x44, 0x01] and replace the original proof value header bytes. The actual proofValue is left as is and the invalid header bytes are encoded as a base64 url with the prefix u.

Additionally:

  • DRYs up an assertion on base proof proofValues into a single assertion
  • Changes several async function related to decoding bs58 and bs64 strings into non-async functions
  • Changes relevant helpers and assertions into non-async functions if the function only depended on a bs decode function previously.

@aljones15 aljones15 self-assigned this Nov 17, 2024
@aljones15 aljones15 changed the base branch from main to add-cbor-tests-sd November 17, 2024 18:01
@aljones15 aljones15 changed the title Add 3 sd proof value tests Add 3 ecdsa-sd-2023 proof value tests Nov 17, 2024
@aljones15 aljones15 marked this pull request as ready for review November 17, 2024 18:04
@aljones15 aljones15 changed the title Add 3 ecdsa-sd-2023 proof value tests Add 3 ecdsa-sd-2023 proof value tests (D -> C) Nov 17, 2024
Comment on lines +190 to +200
`Expected VC from issuer ${name} to have an ' +
'"ecdsa-sd-2023" proof`).to.exist;
expect(
proof.proofValue,
`Expected VC from issuer ${name} to have a ' +
'"proof.proofValue"`
).to.exist;
expect(
proof.proofValue,
`Expected VC "proof.proofValue" from issuer ${name} to be ` +
'a string.'
Copy link
Member

@TallTed TallTed Nov 18, 2024

Choose a reason for hiding this comment

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

Either all Expect ... clauses should end with a fullstop ., or none should. I prefer the former.

I also note mixed up use of ' and `. I strongly advise that these be used consistently, especially but not only in any given statement.

I'm not going to make all the suggestions the above would call for, unless you request that I do so. (I'm betting that changes will be needed throughout the documents touched by this PR, and possibly across other documents.)

Suggested change
`Expected VC from issuer ${name} to have an ' +
'"ecdsa-sd-2023" proof`).to.exist;
expect(
proof.proofValue,
`Expected VC from issuer ${name} to have a ' +
'"proof.proofValue"`
).to.exist;
expect(
proof.proofValue,
`Expected VC "proof.proofValue" from issuer ${name} to be ` +
'a string.'
'Expected VC from issuer ${name} to have an ' +
'"ecdsa-sd-2023" proof.').to.exist;
expect(
proof.proofValue,
'Expected VC from issuer ${name} to have a ' +
'"proof.proofValue".'
).to.exist;
expect(
proof.proofValue,
'Expected VC "proof.proofValue" from issuer ${name} to be ' +
'a string.'

Comment on lines +207 to +208
`Expected "proof.proofValue" to start with u received ` +
`${proof.proofValue[0]}`).to.be.true;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`Expected "proof.proofValue" to start with u received ` +
`${proof.proofValue[0]}`).to.be.true;
'Expected "proof.proofValue" to start with u received ' +
'${proof.proofValue[0]}.').to.be.true;

Comment on lines +209 to +212
// now test the encoding which is bs64 url no pad for this suite
expect(
shouldBeBs64UrlNoPad(proof.proofValue),
'Expected "proof.proofValue" to be bs64 url no pad encoded.'
Copy link
Member

Choose a reason for hiding this comment

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

When aimed at humans, best to use the full string for "bs64 url no pad encoded".

@BigBlueHat BigBlueHat closed this Oct 28, 2025
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.

4 participants