Skip to content

Conversation

@rustyrussell
Copy link
Collaborator

Needs:

  1. Implementation.
  2. Test vectors
  3. Review

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Copy link
Contributor

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

Some minor comments from a first pass. Should have more feedback once we attempt to implement this.

* [`bip340sig`:`sig`]
1. type: 242 (`preimage`)
2. data:
* [`32*byte`:`premage`]
Copy link
Contributor

Choose a reason for hiding this comment

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

s/premage/preimage


The non-signature elements of a payer proof are identical to the
`invoice` tlv_stream, with the exception that `invreq_metadata` cannot
be included. Various fields are omitted for privacy: numbers
Copy link
Contributor

Choose a reason for hiding this comment

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

s/are/may be


A writer of a payer_proof:
- MUST NOT include `invreq_metadata`.
- MUST include `invreq_payer_id`, `invoice_payment_hash`, `invoice_node_id`, `signature` and (if present) `invoice_features` from the invoice.
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you include a rationale for requiring invoice_features?

- `omitted_tlvs` contains 0.
- `omitted_tlvs` contains signature TLV element number (240 through 1000 inclusive).
- `omitted_tlvs` contains the number of an included TLV field.
- `omitted_tlvs` contains more than one number larger than the largest included non-signature TLV element.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand this restriction. Why can't more than one such TLV be omitted? The example contradicts this (41 and 42), IIUC.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops, this was from an earlier draft, and should be removed!


Note that the signature TLV 250 is not included in the merkle tree.

`leaf_hashes` contains the nonce hashes for the present non-signature TLVS:
Copy link
Contributor

Choose a reason for hiding this comment

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

s/TLVS/TLVs

1. type: 250 (`payer_signature`)
2. data:
* [`bip340sig`:`sig`]
* [`...*utf8`:`note`]
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably, this may be empty?

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.

2 participants