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

RPC GetPaymentsStream #345

Merged
merged 5 commits into from
Oct 4, 2024
Merged

Conversation

sekulicd
Copy link
Collaborator

@sekulicd sekulicd commented Oct 3, 2024

Add new RPC GetPaymentsStream to enable accurate payment history reconstruction

This commit introduces the GetPaymentsStream RPC to the ArkService API, allowing clients to subscribe to payment events and reliably recreate the payment history.

This change addresses issues identified in PR #338, where we found that it was not possible to precisely reconstruct all payment (transaction) events solely on the client side. Specifically, we faced challenges in recognizing when boarding transactions were claimed due to race conditions between scanning the boarding transaction's spent status via the explorer and receiving the same event in the round transaction.

By providing this new RPC, clients can now receive real-time updates on payment events, ensuring they can accurately and consistently track payment histories without relying on potentially inconsistent client-side event reconstruction.

@tiero @altafan @louisinger @bordalix please review.

This introduces a new feature to the ArkService API that allows clients to subscribe to payment events. Here's a breakdown of the changes:

1. **OpenAPI Specification (`service.swagger.json`):**
   - A new endpoint `/v1/payments` is added to the API, supporting a `GET` operation for streaming payment events.
   - New definitions `v1GetPaymentsStreamResponse`, `v1RoundPayment`, and `v1AsyncPayment` are added to describe the structure of the streaming responses.

2. **Protobuf Definition (`service.proto`):**
   - Added a new RPC method `GetPaymentsStream` that streams `GetPaymentsStreamResponse` messages.
   - Defined new message types: `GetPaymentsStreamRequest`, `GetPaymentsStreamResponse`, `RoundPayment`, and `AsyncPayment`.

3. **Generated Protobuf Code (`service.pb.go`, `service.pb.gw.go`, `service_grpc.pb.go`):**
   - The generated code is updated to include the new RPC method and message types.
   - The gateway code includes functions to handle HTTP requests and responses for the new streaming endpoint.

4. **Application Logic (`covenant.go`, `covenantless.go`):**
   - New payment events channels are introduced (`paymentEventsCh`).
   - Payment events are propagated to these channels when a round is finalized or an async payment is completed.
   - New event types `RoundPaymentEvent` and `AsyncPaymentEvent` are defined, implementing a `PaymentEvent` interface.

5. **gRPC Handlers (`arkservice.go`):**
   - Added logic to handle `GetPaymentsStream` requests and manage payment listeners.
   - A new goroutine is started to listen to payment events and forward them to active listeners.

Overall, this patch extends the ArkService to support real-time streaming of payment events, allowing clients to receive updates on both round payments and async payments as they occur.
@tiero
Copy link
Member

tiero commented Oct 3, 2024

We should not user Payments, but more general Transactions

altafan and others added 2 commits October 3, 2024 17:21
* Move sending event to updateVtxoSet

* Use generics and parsers
@sekulicd
Copy link
Collaborator Author

sekulicd commented Oct 3, 2024

We should not user Payments, but more general Transactions

@tiero i am not sure about this, Round and Async payment are in fact transactions but inside there can be multiple "payments" where multiple users could send/receive multiple coins(vtxos), also we already name in/outs pairs in Round domain as Payments and adding additional name to the spec can be bit confusing.

Comment on lines 95 to 99
rpc GetPaymentsStream(GetPaymentsStreamRequest) returns (stream GetPaymentsStreamResponse) {
option (google.api.http) = {
get: "/v1/payments"
};
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
rpc GetPaymentsStream(GetPaymentsStreamRequest) returns (stream GetPaymentsStreamResponse) {
option (google.api.http) = {
get: "/v1/payments"
};
}
rpc GetTransactionsStream(GetTransactionsStreamRequest) returns (stream GetTransactionsStreamResponse) {
option (google.api.http) = {
get: "/v1/transactions"
};
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@altafan altafan merged commit d37af7d into ark-network:master Oct 4, 2024
6 checks passed
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