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

fix: verify payment #892

Merged
merged 2 commits into from
Jan 14, 2025
Merged

fix: verify payment #892

merged 2 commits into from
Jan 14, 2025

Conversation

volodymyr-basiuk
Copy link
Contributor

@volodymyr-basiuk volodymyr-basiuk commented Jan 13, 2025

  • The IsPaymentDone contract call should pass the signer address (the one recovered from the signature) as the first parameter, not the Recipient address.
  • Refactor the payment verification process to simplify it.

@volodymyr-basiuk volodymyr-basiuk requested review from a team as code owners January 13, 2025 16:09
@volodymyr-basiuk volodymyr-basiuk changed the base branch from main to develop January 13, 2025 16:10
Copy link
Contributor

@x1m3 x1m3 left a comment

Choose a reason for hiding this comment

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

LGTM.

I jut left a small suggestion about a name but feel free to ignore it

// paymentOptionConfigItem finds the payment option config item used to pay using the payment id stored in PaymentRequest database
func (p *payment) paymentOptionConfigItem(ctx context.Context, issuerDID w3c.DID, item *domain.PaymentRequestItem) (*domain.PaymentOptionConfigItem, error) {
paymentReq, err := p.paymentsStore.GetPaymentRequestByID(ctx, issuerDID, item.PaymentRequestID)
func handleTransaction(
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rename this function to something like handlePaymentTransaction or handleSomethingTransaction to avoid future collisions with other similar functions in package.

But we could postpone this renaming while we don't have this problem,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, renamed

@volodymyr-basiuk volodymyr-basiuk merged commit e799297 into develop Jan 14, 2025
3 checks passed
@volodymyr-basiuk volodymyr-basiuk deleted the fix/verify-payment branch January 14, 2025 09:17
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