Skip to content

Create columns to track settlement gas info - #4697

Merged
jmg-duarte merged 3 commits into
mainfrom
jmgd/gas_settlement
Aug 3, 2026
Merged

Create columns to track settlement gas info#4697
jmg-duarte merged 3 commits into
mainfrom
jmgd/gas_settlement

Conversation

@jmg-duarte

Copy link
Copy Markdown
Contributor

Description

#4540 requires that the migration is done first to avoid issues when upping it in K8s

This PR adds the settlement gas info columns to the settlement table.

Changes

  • Adds gas_used column to settlement table
  • Adds effective_gas_price to settlement table

@jmg-duarte
jmg-duarte requested a review from a team as a code owner August 3, 2026 14:52
@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown

Reminder: Please update the DB Readme and comment whether migrations are reversible (include rollback scripts if applicable).

  • If creating new tables, update the tables list.
  • When adding a new index, consider using CREATE INDEX CONCURRENTLY for tables involved in the critical execution path.
  • For breaking changes, remember that during rollout k8s starts the new autopilot, runs the Flyway migration, and only then shuts down the old pod. That overlap means the previous version can still be processing requests on the migrated schema, so make it compatible first and ship the breaking DB change in the following release.

Caused by:

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @jmg-duarte's task in 1m 7s —— View job


PR Review: Settlement gas info columns

  • Read PR changes
  • Review migration correctness & conventions
  • Check DB readme / tables list requirements
  • Post findings

The migration looks solid — additive nullable columns with numeric(78,0), which matches the u256 convention used throughout the schema (and the existing gas_used numeric(78,0) in V048__create_settlement_rewards.sql). Since the columns are nullable and unreferenced by the old code, it's safe for the k8s rollout overlap, which aligns with the "migration first" strategy described in the PR.

Two small points posted inline:

  • Update the DB README — the settlements table docs don't yet list gas_used / effective_gas_price (per the bot reminder).
  • Stray ) in the comment after "observer".

Neither is blocking.

Comment thread database/sql/V116__settlement_gas_cost.sql Outdated
Comment on lines +9 to +11
ALTER TABLE settlements
ADD COLUMN gas_used numeric(78, 0),
ADD COLUMN effective_gas_price numeric(78, 0);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The bot reminder asks to update the DB README when adding columns. The settlements table docs still don't list gas_used / effective_gas_price — worth adding the two rows so the schema doc stays in sync.

Type/naming look good: numeric(78,0) matches the u256 convention used elsewhere (and the existing gas_used numeric(78,0) in V048__create_settlement_rewards.sql), and nullable additive columns are backward-compatible with the k8s rollout overlap.

jmg-duarte and others added 2 commits August 3, 2026 15:55
Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

@MartinquaXD MartinquaXD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably makes sense to keep the columns separately for higher flexibility but do we currently plan on using those values separately?

@jmg-duarte jmg-duarte added the hotfix Labels PRs that should be applied into production right away label Aug 3, 2026
@jmg-duarte
jmg-duarte added this pull request to the merge queue Aug 3, 2026
Merged via the queue into main with commit 2665a2b Aug 3, 2026
22 checks passed
@jmg-duarte
jmg-duarte deleted the jmgd/gas_settlement branch August 3, 2026 15:57
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 3, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

hotfix Labels PRs that should be applied into production right away

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants