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

Remove "latest_settlement_block" from API #3151

Merged
merged 5 commits into from
Dec 19, 2024
Merged

Conversation

sunce86
Copy link
Contributor

@sunce86 sunce86 commented Dec 9, 2024

Description

Recently we introduced a new database table to store all auction objects -> competition_auctions table. Note that this table does not contain field latest_settlement_block. The reason is that this field is actually deductible from other data in the database -> it's even deductible from blockchain. For a given block X, you can always know what is the latest block before block X where the last settlement was settled.

Moreover, latest_settlement_block is now only used internally in solvable orders cache component of autopilot, so it's not actually a core autopilot information.

This change opens the door to unify auction and competition_auctions table with final goal of removing auction table completely. Removes tech debt.

Changes

  • Remove latest_settlement_block field from api and autopilot

How to test

All tests passing.
If you are worried about a transition period, removing a field shouldn't be problematic for deserializing old object containing latest_settlement_block.

The only concern is that solvers needs to adjust their deserialize logic so it doesn't break. Will be notified in advance once approvals are collected.

@sunce86 sunce86 self-assigned this Dec 9, 2024
@sunce86 sunce86 requested a review from a team as a code owner December 9, 2024 10:58
Copy link

github-actions bot commented Dec 9, 2024

Reminder: Please consider backward compatibility when modifying the API specification.
If breaking changes are unavoidable, ensure:

  • You explicitly pointed out breaking changes.
  • You communicate the changes to affected teams.
  • You provide proper versioning and migration mechanisms.

Caused by:

@m-lord-renkse
Copy link
Contributor

What would happen when calling load_most_recent() or fetch() from the persistence auction? wouldn't it crash because it will have an unexpected field? 🤔

@sunce86
Copy link
Contributor Author

sunce86 commented Dec 9, 2024

What would happen when calling load_most_recent() or fetch() from the persistence auction? wouldn't it crash because it will have an unexpected field? 🤔

If you refer to this line, this would pass.

Copy link
Contributor

@m-lord-renkse m-lord-renkse left a comment

Choose a reason for hiding this comment

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

Approved assuming this will be communicated to solvers on time.

Copy link
Contributor

@squadgazzz squadgazzz left a comment

Choose a reason for hiding this comment

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

LG

@sunce86 sunce86 enabled auto-merge (squash) December 19, 2024 08:51
@sunce86 sunce86 merged commit fe99739 into main Dec 19, 2024
11 checks passed
@sunce86 sunce86 deleted the latest_settlement_block branch December 19, 2024 08:57
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants