Skip to content
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

New payment verification mechanism #1119

Merged
merged 3 commits into from
Mar 10, 2021
Merged

Conversation

Wiezzel
Copy link

@Wiezzel Wiezzel commented Mar 4, 2021

Previous payment verification method was susceptible to tx hash re-using/stealing attack. A new mechanism has been implemented that requires payment drivers to sign a message with payment details with the same key that has been used to sign the blockchain transaction.

Outdated GetTransactionBalance endpoint has been replaced with two new endpoints: SignPayment and VerifySignature. The new endpoints have been implemented in dummy and base driver crates.

Task description: PAY-53

Notice: erc-20 has been intentionally omitted because the implementation in base driver crate should covert it automatically once it is refactored to use that crate.


Testing instructions: Run invoice_flow and debit_note_flow with dummy and zksync drivers.

@Wiezzel Wiezzel self-assigned this Mar 4, 2021
@Wiezzel Wiezzel requested a review from a team March 4, 2021 16:37
Copy link
Contributor

@maaktweluit maaktweluit left a comment

Choose a reason for hiding this comment

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

LGTM!

One possibly "breaking" comment, please have a look, will approve on reply / fix

core/payment-driver/base/src/driver.rs Outdated Show resolved Hide resolved
Previous payment verification method was susceptible to tx hash
re-using/stealing attack. A new mechanism has been implemented that
requires payment drivers to sign a message with payment details with
the same key that has been used to sign the blockchain transaction.

Outdated `GetTransactionBalance` endpoint has been replaced with two
new endpoints: `SignPayment` and `VerifySignature`. The new endpoints
have been implemented in `dummy` and `base` driver crates.

Signed-off-by: Adam Wierzbicki <awierzbicki@golem.network>
maaktweluit
maaktweluit previously approved these changes Mar 8, 2021
Copy link
Contributor

@maaktweluit maaktweluit left a comment

Choose a reason for hiding this comment

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

LGTM!

@Wiezzel
Copy link
Author

Wiezzel commented Mar 9, 2021

@maaktweluit Plz, approve again

Copy link
Contributor

@pnowosie pnowosie left a comment

Choose a reason for hiding this comment

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

👍

@Wiezzel Wiezzel enabled auto-merge March 10, 2021 09:39
@Wiezzel Wiezzel merged commit 15f263e into master Mar 10, 2021
@Wiezzel Wiezzel deleted the wiezzel/payment_signature branch March 10, 2021 10:54
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.

3 participants