tss: Webhook handler service #44

gouthamp-stellar · 2024-09-21T22:17:34Z

What

This is TSS's Webhook Handler Service as described in this doc:
https://docs.google.com/document/d/1xcX86-w8lwT_60flCuYBK9X9co-108-5X6D9e5epw0U/edit#heading=h.76emp0zcbvdy

Why

This service is responsible for sending completed transactions to the client

Known limitations

N/A

Issue that this PR addresses

https://app.asana.com/0/1207947010297920/1208138733594326

Checklist

PR Structure

It is not possible to break this PR down into smaller PRs.
This PR does not mix refactoring changes with feature changes.
This PR's title starts with name of package that is most changed in the PR, or all if the changes are broad or impact many packages.

Thoroughness

This PR adds tests for the new functionality or fixes.
All updated queries have been tested (refer to this check if the data set returned by the updated query is expected to be same as the original one).

Release

This is not a breaking change.
This is ready to be tested in development.
The new functionality is gated with a feature flag if this is not ready for production.

adding semicolons

…ransaction_hash ...also remove outgoing/incoming_status and have one status column

commit fb807aa Author: gouthamp-stellar <[email protected]> Date: Tue Sep 3 10:28:49 2024 -0700 remove the index on try_transaction_xdr and add column/index on try_transaction_hash ...also remove outgoing/incoming_status and have one status column commit 6fc0dc2 Author: gouthamp-stellar <[email protected]> Date: Tue Sep 3 08:58:46 2024 -0700 missing , commit a9cf4e3 Author: gouthamp-stellar <[email protected]> Date: Tue Sep 3 01:55:23 2024 -0700 make hash primary key instead of xdr commit c0f9d32 Author: gouthamp-stellar <[email protected]> Date: Fri Aug 30 15:18:27 2024 -0700 moving all migrations to one file commit 2de9898 Author: gouthamp-stellar <[email protected]> Date: Fri Aug 30 15:16:53 2024 -0700 adding semicolons adding semicolons commit 373c71a Author: gouthamp-stellar <[email protected]> Date: Fri Aug 30 15:12:24 2024 -0700 update commit 3f9f9f0 Author: gouthamp-stellar <[email protected]> Date: Fri Aug 30 15:12:00 2024 -0700 remove empty lines commit 9920f48 Author: gouthamp-stellar <[email protected]> Date: Fri Aug 30 15:06:40 2024 -0700 TSS tables and the channel interface commit a58d519 Author: gouthamp-stellar <[email protected]> Date: Fri Aug 30 10:55:22 2024 -0700 tables and interfaces for TSS

…and builds a transaction out of it

…he Payload

gsoarescosta · 2024-10-10T19:37:05Z

cmd/utils/tss_options.go

+func WebhookHandlerServiceChannelMinWaitBtwnRetriesMSOption(configKey *int) *config.ConfigOption {
+	return &config.ConfigOption{
+		Name:        "webhook-service-channel-min-wait-between-retries",
+		Usage:       "The minumum amout of time to wait before repining the webhook url",


Is this in seconds, right?

Suggested change

Usage: "The minumum amout of time to wait before repining the webhook url",

Usage: "The minimum amount of time to wait before resending the request to the webhook url",

Corrected this typo. As for wait times, typically from what I've seen, and done in the past, in exponential backoff implementations is a wait time in the order of milliseconds so I'm doing the same here. Also we have to remember that this is being done inside a task in a thread pool, which typically should not be long running, so milliseconds seems appropriate to me

gsoarescosta · 2024-10-10T19:43:18Z

internal/tss/channels/webhook_channel.go

+	}
+	var i int
+	for i = 0; i < p.MaxRetries; i++ {
+		resp, err := p.HTTPClient.Post(payload.WebhookURL, "application/json", bytes.NewBuffer(jsonData))


Here we are just sending the data as a JSON body payload to the webhook URL, right? There's no authentication nor any custom header.
I'm imagining here that maybe we should support some kind of Signature header, to add a security layer in the communication between the wallet-backend and the customer application.
What do you think?

I actually do like the idea of adding a security layer, but want to give it some more thought. I'm working on a doc for tss v2 ideas and I'll make sure to add this to it. I think that we should be ok not having auth for now because customer applications will deploy their own wallet backends so there's some inherent trust there already

gsoarescosta · 2024-10-16T20:32:54Z

internal/tss/utils/helpers.go

+	response := tss.TSSResponse{}
+	response.TransactionHash = payload.TransactionHash
+	if payload.RpcSubmitTxResponse.Status.Status() != "" {
+		response.Status = string(payload.RpcSubmitTxResponse.Status.Status())


This .Status() method already returns a string, so you don't need to parse it here

Suggested change

response.Status = string(payload.RpcSubmitTxResponse.Status.Status())

response.Status = payload.RpcSubmitTxResponse.Status.Status()

gsoarescosta · 2024-10-16T20:34:52Z

internal/tss/channels/webhook_channel.go

+		MaxRetries:           cfg.MaxRetries,
+		MinWaitBtwnRetriesMS: cfg.MinWaitBtwnRetriesMS,
+	}
+


Suggested change

gsoarescosta · 2024-10-16T20:36:32Z

cmd/utils/tss_options.go

+func WebhookHandlerServiceChannelMaxBufferSizeOption(configKey *int) *config.ConfigOption {
+	return &config.ConfigOption{
+		Name:        "webhook-service-channel-max-buffer-size",
+		Usage:       "Set the buffer size of the webhook serive channel.",


Suggested change

Usage: "Set the buffer size of the webhook serive channel.",

Usage: "Set the buffer size of the webhook service channel.",

gsoarescosta · 2024-10-16T20:36:46Z

cmd/utils/tss_options.go

+func WebhookHandlerServiceChannelMaxWorkersOptions(configKey *int) *config.ConfigOption {
+	return &config.ConfigOption{
+		Name:        "webhook-service-channel-max-workers",
+		Usage:       "Set the max number of workers for the webhook serive channel.",


Suggested change

Usage: "Set the max number of workers for the webhook serive channel.",

Usage: "Set the max number of workers for the webhook service channel.",

gsoarescosta · 2024-10-16T20:37:19Z

cmd/utils/tss_options.go

+func WebhookHandlerServiceChannelMinWaitBtwnRetriesMSOption(configKey *int) *config.ConfigOption {
+	return &config.ConfigOption{
+		Name:        "webhook-service-channel-min-wait-between-retries",
+		Usage:       "The minumum amout of time to wait before resending the payload to the webhook url",


Suggested change

Usage: "The minumum amout of time to wait before resending the payload to the webhook url",

Usage: "The minimum amount of time (in milliseconds) to wait before resending the payload to the webhook url",

gsoarescosta · 2024-10-16T20:38:55Z

cmd/serve.go

+		utils.WebhookHandlerServiceChannelMaxWorkersOptions(&cfg.WebhookHandlerServiceChannelMaxWorkers),
+		utils.WebhookHandlerServiceChannelMaxRetriesOption(&cfg.WebhookHandlerServiceChannelMaxRetries),
+		utils.WebhookHandlerServiceChannelMinWaitBtwnRetriesMSOption(&cfg.WebhookHandlerServiceChannelMinWaitBtwnRetriesMS),
+


Suggested change

gsoarescosta · 2024-10-16T20:45:08Z

internal/tss/channels/webhook_channel.go

+	})
+}
+
+func (p *webhookPool) Receive(payload tss.Payload) {


Can we add a test to validate this method errors? I see that you have a test that covers the "happy path", just wondering here if it's nice do validate these errors too

gsoarescosta

Just approved, just left a few comments about some improvements that are nice to have, imo :)

gouthamp-stellar added 30 commits August 30, 2024 10:55

tables and interfaces for TSS

a58d519

TSS tables and the channel interface

9920f48

remove empty lines

3f9f9f0

update

373c71a

adding semicolons

2de9898

adding semicolons

moving all migrations to one file

c0f9d32

make hash primary key instead of xdr

a9cf4e3

missing ,

6fc0dc2

remove the index on try_transaction_xdr and add column/index on try_t…

fb807aa

…ransaction_hash ...also remove outgoing/incoming_status and have one status column

commit #2

d774f2a

changing from RpcIngestTxResponse -> RpcGetIngestTxResponse

6b04d62

Merge branch 'tss_tables_interfaces' into transaction_module

f2fefef

latest changes

b441a31

add tests for ValidateOptions

02c7de3

changes based on comments

f9a5611

Merge branch 'main' into tss_tables_interfaces

32bd6ce

Merge branch 'tss_tables_interfaces' into transaction_module

5c7db27

latest changes based on changes to interface

c3e8de9

string -> RPCTXStatus

82b12fb

Merge branch 'tss_tables_interfaces' into transaction_module

9a9c9be

adding a transaction_builder.go which takes a list of operation xdrs …

0662da9

…and builds a transaction out of it

moving transaction_service to the utils dir

8f76d61

upper case Channel methods

456b71f

Merge branch 'tss_tables_interfaces' into transaction_module

7974aa9

latest changes

5bdf8f2

p.txService.NetworkPassPhrase()

3890bfe

last commit before writing unit tests

e87c6d6

Making the transaction service more injectible and adding fields to t…

c1d3786

…he Payload

typo

01ed3eb

gouthamp-stellar added 10 commits September 30, 2024 15:46

removing test case that is not relevant anymore

b4f6052

Merge branch 'rpc_caller_service' into error_handler_service

3d6746e

changes based on prev pr comments

9be1beb

remove fmt.Println

0df3537

Merge branch 'error_handler_service' into webhook_handler_service

dc9a363

merge latest error_handler_service branch + small changes

8c445f4

%s -> %w

2caf65b

U -> u

ab2a546

Merge branch 'error_handler_service' into webhook_handler_service

277b970

variable for channel name

d576a25

gouthamp-stellar requested a review from daniel-burghardt October 1, 2024 18:10

Base automatically changed from error_handler_service to main October 9, 2024 03:54

gouthamp-stellar added 2 commits October 9, 2024 16:33

Merge branch 'main' into webhook_handler_service

d0fa39d

merge main

44cb3ec

gouthamp-stellar requested review from CaioTeixeira95 and gsoarescosta October 9, 2024 23:41

gsoarescosta reviewed Oct 10, 2024

View reviewed changes

gouthamp-stellar added 2 commits October 15, 2024 11:45

typo in desc string

ad392d8

millisecond wait time

dbe9629

gsoarescosta reviewed Oct 16, 2024

View reviewed changes

gsoarescosta approved these changes Oct 16, 2024

View reviewed changes

gouthamp-stellar merged commit 6553fa9 into main Oct 22, 2024
5 checks passed

gouthamp-stellar deleted the webhook_handler_service branch October 22, 2024 23:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tss: Webhook handler service #44

tss: Webhook handler service #44

gouthamp-stellar commented Sep 21, 2024 •

edited

Loading

gsoarescosta Oct 10, 2024

gouthamp-stellar Oct 15, 2024 •

edited

Loading

gsoarescosta Oct 10, 2024

gouthamp-stellar Oct 15, 2024

gsoarescosta Oct 16, 2024

gsoarescosta Oct 16, 2024

gsoarescosta Oct 16, 2024

gsoarescosta Oct 16, 2024

gsoarescosta Oct 16, 2024 •

edited

Loading

gsoarescosta Oct 16, 2024

gsoarescosta Oct 16, 2024

gsoarescosta left a comment

	Usage: "The minumum amout of time to wait before repining the webhook url",
	Usage: "The minimum amount of time to wait before resending the request to the webhook url",

	response.Status = string(payload.RpcSubmitTxResponse.Status.Status())
	response.Status = payload.RpcSubmitTxResponse.Status.Status()

	Usage: "Set the buffer size of the webhook serive channel.",
	Usage: "Set the buffer size of the webhook service channel.",

	Usage: "Set the max number of workers for the webhook serive channel.",
	Usage: "Set the max number of workers for the webhook service channel.",

	Usage: "The minumum amout of time to wait before resending the payload to the webhook url",
	Usage: "The minimum amount of time (in milliseconds) to wait before resending the payload to the webhook url",

tss: Webhook handler service #44

tss: Webhook handler service #44

Conversation

gouthamp-stellar commented Sep 21, 2024 • edited Loading

What

Why

Known limitations

Issue that this PR addresses

Checklist

PR Structure

Thoroughness

Release

Choose a reason for hiding this comment

gouthamp-stellar Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsoarescosta Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsoarescosta left a comment

Choose a reason for hiding this comment

gouthamp-stellar commented Sep 21, 2024 •

edited

Loading

gouthamp-stellar Oct 15, 2024 •

edited

Loading

gsoarescosta Oct 16, 2024 •

edited

Loading