-
Couldn't load subscription status.
- Fork 199
[Access] Add schedule tx support to rest api #8052
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
base: peter/7830-scheduled-tx-endpoints2
Are you sure you want to change the base?
[Access] Add schedule tx support to rest api #8052
Conversation
| } | ||
|
|
||
| for _, tt := range tests { | ||
| func TestBenchmarkURLToRoute(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the new version completes ~25% faster (1.15s vs 1.55s)
…eduled-tx-endpoints-rest
…eduled-tx-endpoints-rest
…eduled-tx-endpoints-rest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| return err | ||
| } | ||
|
|
||
| type GetScheduledTransaction struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add small godoc for GetScheduledTransaction, GetScheduledTransactionResult and its constructors?
| return response, nil | ||
| } | ||
|
|
||
| func isTransactionID(raw string) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please a couple of words to the godoc for isTransactionID?
| }) | ||
| } | ||
|
|
||
| func TestGetScheduledTransactions(t *testing.T) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests look good overall! The only things I’d suggest are adding godocs for the test functions and helpers, and using .Once() to verify backend mock calls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - just a few minor documentation-related suggestions
|
|
||
| const idQuery = "id" | ||
|
|
||
| // GetTransactionByID gets a transaction by requested ID. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // GetTransactionByID gets a transaction by requested ID. | |
| // GetTransactionByID gets a transaction by requested ID. | |
| // The ID may be either: | |
| // 1. the hex-encoded 32-byte hash of a user-submitted transaction, or | |
| // 2. the integral system-assigned identifier of a scheduled transaction |
| return response, nil | ||
| } | ||
|
|
||
| // GetTransactionResultByID retrieves transaction result by the transaction ID. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // GetTransactionResultByID retrieves transaction result by the transaction ID. | |
| // GetTransactionResultByID retrieves transaction result by the transaction ID. | |
| // The ID may be either: | |
| // 1. the hex-encoded 32-byte hash of a user-submitted transaction, or | |
| // 2. the integral system-assigned identifier of a scheduled transaction |
| // isTransactionID returns true if the provided string is a valid flow.Identifier indicating it is a | ||
| // transaction ID. | ||
| func isTransactionID(raw string) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // isTransactionID returns true if the provided string is a valid flow.Identifier indicating it is a | |
| // transaction ID. | |
| func isTransactionID(raw string) bool { | |
| // isTransactionID returns true if the provided string is a valid hex-encoded 32-byte flow.Identifier indicating it is a transaction ID. | |
| // In particular, this method returns false if the input is a *scheduled transaction* ID. | |
| func isTransactionID(raw string) bool { |
This PR add support for querying scheduled and system transactions using the existing
/transactions/{id}and/transaction_results/{id}endpoints.