Conversation
There was a problem hiding this comment.
Pull request overview
Updates the new_max_fee (fee-bump max fee) field’s type to use a wider integer representation, likely to avoid overflow/truncation issues when handling larger fee values.
Changes:
- Change
TransactionOutput.NewMaxFeefromuint32toint64in the JSON schema. - Update
TransformTransactionto populateNewMaxFeeasint64. - Adjust the Parquet schema tag for
new_max_fee.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/transform/transaction.go | Writes NewMaxFee as int64 when transforming fee-bump transactions. |
| internal/transform/schema.go | Changes TransactionOutput.NewMaxFee type to int64 in the JSON-facing schema. |
| internal/transform/schema_parquet.go | Modifies Parquet field metadata for new_max_fee (removes UINT_64 logical type). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FeeAccountMuxed string `json:"fee_account_muxed,omitempty"` | ||
| InnerTransactionHash string `json:"inner_transaction_hash,omitempty"` | ||
| NewMaxFee uint32 `json:"new_max_fee,omitempty"` | ||
| NewMaxFee int64 `json:"new_max_fee,omitempty"` |
There was a problem hiding this comment.
Changing new_max_fee from uint32 to int64 is a schema/API breaking change for any consumers that deserialize TransactionOutput (and any downstream BigQuery schema expectations). If this is intentional, it should be accompanied by a documented migration/compatibility plan (e.g., schema version bump, backfill/reload notes, or explicit downstream consumer updates).
| NewMaxFee int64 `json:"new_max_fee,omitempty"` | |
| NewMaxFee uint32 `json:"new_max_fee,omitempty"` |
| FeeAccountMuxed string `parquet:"name=fee_account_muxed, type=BYTE_ARRAY, convertedtype=UTF8, encoding=PLAIN_DICTIONARY"` | ||
| InnerTransactionHash string `parquet:"name=inner_transaction_hash, type=BYTE_ARRAY, convertedtype=UTF8, encoding=PLAIN_DICTIONARY"` | ||
| NewMaxFee int64 `parquet:"name=new_max_fee, type=INT64, convertedtype=UINT_64"` | ||
| NewMaxFee int64 `parquet:"name=new_max_fee, type=INT64"` |
There was a problem hiding this comment.
This changes the Parquet schema for new_max_fee by removing the unsigned logical type (convertedtype=UINT_64) while similar fee fields (e.g., max_fee at internal/transform/schema_parquet.go:38) still use UINT_64. If downstream readers expect new_max_fee to be unsigned (or rely on the previous Parquet schema), this will be a breaking change; consider keeping convertedtype=UINT_64 for consistency, or updating all fee-related fields + consumers together with a clear migration.
| NewMaxFee int64 `parquet:"name=new_max_fee, type=INT64"` | |
| NewMaxFee int64 `parquet:"name=new_max_fee, type=INT64, convertedtype=UINT_64"` |
PR Checklist
PR Structure
otherwise).
Thoroughness
Release planning
semver, and I've changed the name of the BRANCH to major/_ , minor/_ or patch/* .
What
[TODO: Short statement about what is changing.]
Why
[TODO: Why this change is being made. Include any context required to understand the why.]
Known limitations
[TODO or N/A]