Skip to content

Remove GetAddressTxs #3872

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

Merged
merged 12 commits into from
Apr 10, 2025
Merged

Remove GetAddressTxs #3872

merged 12 commits into from
Apr 10, 2025

Conversation

joshua-kim
Copy link
Contributor

@joshua-kim joshua-kim commented Apr 9, 2025

Why this should be merged

Removes deprecated api to minimize amount of state required for #3732. We have confirmed that there are no downstream users of this api.

How this works

Removes GetAddressTxs + avm tx indexer code.

How this was tested

CI

Need to be documented in RELEASES.md?

Yes

Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
@joshua-kim joshua-kim self-assigned this Apr 9, 2025
@joshua-kim joshua-kim added the cleanup Code quality improvement label Apr 9, 2025
@joshua-kim joshua-kim modified the milestone: v1.12.1 Apr 9, 2025
Signed-off-by: Joshua Kim <[email protected]>
@StephenButtolph
Copy link
Contributor

Could you add the release notes shell to RELEASES.md (based on a prior release like v1.12.2) and include the changes there?

Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
@joshua-kim
Copy link
Contributor Author

Could you add the release notes shell to RELEASES.md (based on a prior release like v1.12.2) and include the changes there?

Added 5e55d87

Signed-off-by: Joshua Kim <[email protected]>
Co-authored-by: Stephen Buttolph <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
@joshua-kim joshua-kim force-pushed the remove-get-address-txs branch from 165b03d to cf09730 Compare April 9, 2025 21:34
Signed-off-by: Joshua Kim <[email protected]>
@joshua-kim joshua-kim force-pushed the remove-get-address-txs branch from cf09730 to a49bf75 Compare April 9, 2025 21:35
Signed-off-by: Joshua Kim <[email protected]>
Signed-off-by: Joshua Kim <[email protected]>
@joshua-kim joshua-kim force-pushed the remove-get-address-txs branch from fb4eb39 to 70407c5 Compare April 9, 2025 21:59
Copy link
Contributor

@StephenButtolph StephenButtolph left a comment

Choose a reason for hiding this comment

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

Last nit then lgtm

Signed-off-by: Joshua Kim <[email protected]>
@joshua-kim joshua-kim changed the base branch from master to branch-v1.12.0 April 10, 2025 02:17
@joshua-kim joshua-kim changed the base branch from branch-v1.12.0 to master April 10, 2025 02:18
@joshua-kim joshua-kim enabled auto-merge April 10, 2025 02:20
@rrazvan1
Copy link
Contributor

rrazvan1 commented Apr 10, 2025

since we are getting rid of the AddressTxsIndexer, does it make sense to also delete the data?

@joshua-kim
Copy link
Contributor Author

joshua-kim commented Apr 10, 2025

since we are getting rid of the AddressTxsIndexer, does it make sense to also delete the data?

It makes sense. I considered doing it as part of this PR but this feature/its apis have been deprecated for 2 years now + we're not aware of anyone using it (we confirmed with Binance/our internal teams, and the indexer defaults to being disabled). IMO it's not worth the effort.

@joshua-kim joshua-kim added this pull request to the merge queue Apr 10, 2025
Merged via the queue into master with commit 2541190 Apr 10, 2025
24 checks passed
@joshua-kim joshua-kim deleted the remove-get-address-txs branch April 10, 2025 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code quality improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants