Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: store total transaction size in blocks table #2204

Merged
merged 3 commits into from
Jan 22, 2025
Merged

Conversation

rafaelcr
Copy link
Collaborator

This is computed by the sum of all confirmed transaction byte sizes, and will be necessary to calculate tenure fullness in fee estimations.

Copy link

github-actions bot commented Jan 22, 2025

Vercel deployment URL: https://stacks-blockchain-8kcg3xjul-hirosystems.vercel.app 🚀

Copy link

codecov bot commented Jan 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@rafaelcr rafaelcr marked this pull request as ready for review January 22, 2025 18:40
@rafaelcr rafaelcr requested a review from janniks January 22, 2025 18:40
Copy link
Collaborator

@janniks janniks left a comment

Choose a reason for hiding this comment

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

Generally looks great!

Only weird to see tx_total_size: 1, in the tests, I'm assuming they don't have a "real" transaction with a correct size?

@rafaelcr
Copy link
Collaborator Author

thanks @janniks , exactly, they're only simulated values for tests

@rafaelcr rafaelcr merged commit ac7c41b into develop Jan 22, 2025
27 checks passed
@rafaelcr rafaelcr deleted the feat/block-size branch January 22, 2025 19:44
Comment on lines +5 to +20
exports.up = pgm => {
pgm.addColumn('blocks', {
tx_total_size: {
type: 'int',
notNull: true,
default: 0,
},
});
pgm.sql(`
UPDATE blocks
SET tx_total_size = (
SELECT SUM(OCTET_LENGTH(raw_tx))
FROM txs
WHERE index_block_hash = blocks.index_block_hash
)
`);
Copy link
Member

Choose a reason for hiding this comment

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

My gut feeling is that this will result in a several hour long migration for most external deployments (e.g. exchanges). Seems like it would be fine to let the new column be nullable and we'd only have this available for new blocks.

blockstack-devops pushed a commit that referenced this pull request Jan 28, 2025
## [8.6.0-beta.1](v8.5.0...v8.6.0-beta.1) (2025-01-28)

### Features

* consider tenure block fullness for transaction fee estimations ([#2203](#2203)) ([396e2ea](396e2ea))
* store total transaction size in blocks table ([#2204](#2204)) ([ac7c41b](ac7c41b))

### Bug Fixes

* make tx_total_size column nullable ([#2207](#2207)) ([77bd2f8](77bd2f8))
@blockstack-devops
Copy link

🎉 This PR is included in version 8.6.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants