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.

Sign up for GitHub

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

Jump to bottom

Improve post addresses transactions #568

Merged
merged 2 commits into from
Oct 3, 2024
Merged

Improve post addresses transactions #568

merged 2 commits into from
Oct 3, 2024

Conversation

tdroxler
Copy link
Member

@tdroxler tdroxler commented Oct 1, 2024

POST /addresses/transactions was returning duplicate tx if multiple addresses of to the body share some common tx.

We also added fromTs and toTs optional query params to that endpoint to ease the pagination work.

@tdroxler tdroxler requested a review from h0ngcha0 October 1, 2024 14:58
@tdroxler
Return distinct transaction when querying txs by addresses
47573ef 
If a tx was associated with 2 addresses and we were requesting the txs
of multiple addresses, if the 2 addresses were there, we were returing
twice the same tx.
@tdroxler
Add time interval query params to POST /addresses/transactions
5c7c1ff
@tdroxler tdroxler force-pushed the improve-post-addresses-transactions branch from 13c80b7 to 5c7c1ff Compare October 2, 2024 07:59
@tdroxler tdroxler changed the base branch from master to latest-tx-info October 2, 2024 07:59
h0ngcha0
h0ngcha0 approved these changes Oct 2, 2024
View reviewed changes
Copy link
Member

@h0ngcha0 h0ngcha0 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@polarker
Copy link
Member

polarker commented Oct 3, 2024

Why this method is POST instead of GET?

@tdroxler
Copy link
Member Author

tdroxler commented Oct 3, 2024

Why this method is POST instead of GET?

You can query transactions for multiple addresses, you pass them in the POST body. As we paginate we don't return more transactionos, limit is the same as the GET for one address.

@polarker
Copy link
Member

polarker commented Oct 3, 2024

Why this method is POST instead of GET?

You can query transactions for multiple addresses, you pass them in the POST body. As we paginate we don't return more transactionos, limit is the same as the GET for one address.

POST is usually used to create a resource

POST is used to send data to a server to create/update a resource.

@tdroxler
Copy link
Member Author

tdroxler commented Oct 3, 2024

POST is usually used to create a resource

POST is used to send data to a server to create/update a resource.

yes, but REST doesn't have a nice way to request multiple items, so it's quite common to use POST for that kind of situation. The other solution is to go with chain of query param: ?address=<addr1>&address=<addr2>, but you have a risk to reach the url length limit, especially with addresses that can be long.

@polarker
Copy link
Member

polarker commented Oct 3, 2024

I keep forgetting that GET does not allow data to be sent to the server. We don't have other choices then.

Base automatically changed from latest-tx-info to master October 3, 2024 12:53
@tdroxler tdroxler merged commit 8eddbdf into master Oct 3, 2024
1 check passed
@tdroxler tdroxler deleted the improve-post-addresses-transactions branch October 3, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@polarker polarker polarker approved these changes

@h0ngcha0 h0ngcha0 h0ngcha0 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tdroxler @polarker @h0ngcha0