-
Notifications
You must be signed in to change notification settings - Fork 136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Payment Proof export + validation #289
Conversation
This is ready for review/comment now |
libwallet/src/api_impl/types.rs
Outdated
#[serde(with = "dalek_ser::dalek_pubkey_serde")] | ||
pub recipient_address: DalekPublicKey, | ||
/// Onion V3 Adddress of recipient | ||
pub recipient_ov3_address: String, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can get ov3 address from the public key and vice-versa, so I would prefer to choose whichever one is more meaningful and not include the other.
If both were included just because we lack the conversion utilities, I'm willing to write those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conversion code is there, I know this seems technically redundant, but I'm thinking more about human-readability here (the ov3 address isn't actually used anywhere). It's currently possible to send directly to an oV3 address, or to send via another means while explicitly specifying the recipient public key, so I had thought to include both in the proof just to ensure nobody looking at the proof would need to take extra conversion steps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking further, we should probably just be displaying the onion V3 address everywhere and converting internally, particularly when specifying an address manually via the send command. Unfortunately that horse has bolted slightly, may be worth accepting the pain of correcting this in v3.1.0?
Going to remove the raw pubkey and prefer the OV3 address exclusively for the user facing address. Going to merge this now, and follow up with another PR that cleans up the addresses as per #306 |
* add payment proof struct and deser * rustfmt * adding proof export functions * rustfmt * add payment proof validation function * rustfmt * add RPC version of retrieve_payment_proof + doctest * rustfmt * add verify_proof rpc function and documentation for new functions * rustfmt * add export and verify commands * rustfmt * test + test framework fixes * rustfmt * check whether addresses belong to this wallet, output such when checking * rustfmt * remove raw pubkey address and replace with ov3 address in user-facing contexts * merge from master and rustfmt * doctests
Aims to add functionality to the API to export payment proofs as JSON and validate an exported payment proof, as well as command-line options calling both.
Update on completion:
PaymentProof
struct intended for serialization into JSON for export to file or use via the APIretrieve_payment_proof
andverify_payment_proof
(details in rustdoc documentation, as well as supporting internal implementationsexport_proof
andverify_proof
command line options