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

httphandler: tss api endpoints #54

Merged
merged 145 commits into from
Oct 29, 2024
Merged

httphandler: tss api endpoints #54

merged 145 commits into from
Oct 29, 2024

Conversation

gouthamp-stellar
Copy link
Contributor

@gouthamp-stellar gouthamp-stellar commented Oct 7, 2024

What

TSS API endpoints to

  1. submit transactions
  2. get transaction

Why

For clients of TSS to be able to submit transactions and get a transaction that they submitted

Known limitations

N/A

Issue that this PR addresses

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

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
Copy link
Contributor

@CaioTeixeira95 CaioTeixeira95 left a comment

Choose a reason for hiding this comment

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

Overall, LGTM.

}

type Transaction struct {
Operations []string `json:"operations"`
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend we add validate flag here too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

endpoint := "/tss/transactions"

r := chi.NewRouter()
//r.Get("/tss/{transactionhash}", handler.GetTransaction)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove this line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 206 to 209
assert.Equal(t, http.StatusOK, resp.StatusCode)
assert.Empty(t, getTxResp.Hash)
assert.Empty(t, getTxResp.XDR)
assert.Empty(t, getTxResp.Status)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we could return 404 instead of 200 when the transaction hash doesn't exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment on lines 108 to 111
if err != nil {
httperror.InternalServerError(ctx, "unable to get transaction", err, nil, t.AppTracker).Render(w)
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just wondering, should we log the transaction hash explicitly here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Base automatically changed from populate_pools to main October 23, 2024 23:47
@gouthamp-stellar gouthamp-stellar merged commit 8469e6f into main Oct 29, 2024
5 checks passed
@gouthamp-stellar gouthamp-stellar deleted the tss_api_endpoints branch October 29, 2024 18:06
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