Skip to content
This repository was archived by the owner on Mar 8, 2024. It is now read-only.

Verifiable addresses#17

Merged
nhartner merged 5 commits intomasterfrom
verifiable-addresses
Aug 5, 2020
Merged

Verifiable addresses#17
nhartner merged 5 commits intomasterfrom
verifiable-addresses

Conversation

@nhartner
Copy link
Copy Markdown
Collaborator

@nhartner nhartner commented Aug 3, 2020

High Level Overview of Change

adds functions and types to sign a PayID address using identity or server key and generate JWS. A complete usage looks like this.

Build utils. Launch node repl:

npm install
npm run build
node --experimental-repl-await

Run the following code inside the repl:

jose = require('jose')
payidUtils = require('./dist')

var jwk = await jose.JWK.generate('EC', 'secp256k1')
var alg = 'ES256K'

var payId = 'nhartner$payid.ml'
var address = { paymentNetwork: 'XRPL', environment: 'MAINNET', addressDetailsType: 'CryptoAddressDetails', addressDetails: { address: 'rP3t3JStqWPYd8H88WfBYh3v84qqYzbHQ6' }}

var jws = payidUtils.sign(payId, address, { key: jwk, alg })
console.log(JSON.stringify(jws, null, 2))

payidUtils.verifySignedAddress(payId, jws)

Context of Change

new functions in new files

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • [ X ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

Before / After

Test Plan

Generate and sign payIDs using various combinations of key and algorithms.

@nhartner nhartner requested a review from 0xCLARITY as a code owner August 3, 2020 22:02
Copy link
Copy Markdown
Member

@dino-rodriguez dino-rodriguez left a comment

Choose a reason for hiding this comment

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

This is great Neil. This will be super useful for validating JWS and in general testing out the full-fledged usage of the server implementation.


import { SigningParams } from './verifiable-payid'

import ECKey = JWK.ECKey
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just curious as I haven't seen this before : why are these also import statements instead of constants / variables?

* @param signingParams - The key/alg to use to generate the signature.
* @returns A signed JWS.
*/
export function sign(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is super clean and simple.

* @param signingParams - The key/alg to use to generate the signature.
* @returns A signed JWS.
*/
function signWithIdentityKey(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nice helper abstractions!

}
})

it('Signed PayID returns JWS', async function () {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For tests we've usually been doing GIVEN WHEN THIS comments, not sure if you wanted to continue this pattern here, but imo it makes tests easily human readable.

address: Address,
signingParams: Array<IdentityKeySigningParams | ServerKeySigningParams>,
): GeneralJWS {
// There seems to be a bug with the JOSE library when dealing with multiple signatures + unencoded payloads.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Helpful comment, nice.

@nhartner nhartner merged commit fa4bb26 into master Aug 5, 2020
@nhartner nhartner deleted the verifiable-addresses branch August 5, 2020 20:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants