Skip to content

refactor(blockchain): change default transaction layout to combined and remove related flags from commands#3506

Merged
thiagodeev merged 5 commits intomainfrom
thiagodeev/make-combine-layout-migration-mandatory
Apr 6, 2026
Merged

refactor(blockchain): change default transaction layout to combined and remove related flags from commands#3506
thiagodeev merged 5 commits intomainfrom
thiagodeev/make-combine-layout-migration-mandatory

Conversation

@thiagodeev
Copy link
Copy Markdown
Contributor

@thiagodeev thiagodeev commented Mar 25, 2026

Summary

  • Makes the blocktransactions migration mandatory (no longer opt-in) and switches the default transaction storage layout from per-tx to combined (per-block)
  • Removes the --transaction-combined-layout CLI flag and all associated config/wiring
  • Returns db.ErrKeyNotFound for non-existent blocks in combined layout (consistent error propagation), and adds test coverage for both non-existent blocks and valid
    blocks with no transactions

A follow-up PR will be made to completely remove the old layout code.

Changes

Migration & config cleanup:

  • node/migration.go: WithOptionalWith for the blocktransactions migrator
  • node/node.go: Remove FlagTransactionCombinedLayout constant and TransactionCombinedLayout from Config
  • blockchain/blockchain.go: Change default layout in New() to TransactionLayoutCombined
  • cmd/juno/juno.go + cmd/juno/dbcmd.go: Remove --transaction-combined-layout flag from all commands

Tests:

  • blockchain/blockchain_test.go: Add test for valid block with no transactions (expects success + empty result), and update non-existent/reverted block tests to expect errors
  • core/transaction_layout_test.go: Add NonExistentBlock_* subtests verifying both layouts return errors for missing blocks
  • Some cases in the TestTransactionLayout are being skipped for the old layout since it behaves differently, and we plan to remove it

@thiagodeev thiagodeev changed the base branch from thiagodeev/make-v10-default to main March 25, 2026 02:18
Comment thread blockchain/blockchain.go
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 27, 2026

Codecov Report

❌ Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.28%. Comparing base (499b208) to head (6774432).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
node/migration.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3506      +/-   ##
==========================================
- Coverage   75.48%   75.28%   -0.21%     
==========================================
  Files         386      386              
  Lines       34907    34912       +5     
==========================================
- Hits        26350    26283      -67     
- Misses       6712     6728      +16     
- Partials     1845     1901      +56     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thiagodeev thiagodeev marked this pull request as ready for review March 27, 2026 20:56
Comment thread core/transaction_layout.go
@thiagodeev thiagodeev marked this pull request as draft March 31, 2026 11:22
@thiagodeev thiagodeev added the disable-deploy-test We don't want to run deploy tests with this PR because it might affect our development environment. label Mar 31, 2026
@thiagodeev thiagodeev force-pushed the thiagodeev/make-combine-layout-migration-mandatory branch from e3493a4 to f4b452c Compare April 2, 2026 03:41
@thiagodeev thiagodeev marked this pull request as ready for review April 2, 2026 03:42
@thiagodeev thiagodeev force-pushed the thiagodeev/make-combine-layout-migration-mandatory branch from f4b452c to 3fea15e Compare April 2, 2026 13:01
Comment thread blockchain/blockchain.go
Copy link
Copy Markdown
Contributor

@rodrodros rodrodros left a comment

Choose a reason for hiding this comment

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

Looks good!

Comment thread cmd/juno/juno.go Outdated
@thiagodeev thiagodeev force-pushed the thiagodeev/make-combine-layout-migration-mandatory branch from 3fea15e to aac79fd Compare April 6, 2026 13:16
@thiagodeev thiagodeev removed the disable-deploy-test We don't want to run deploy tests with this PR because it might affect our development environment. label Apr 6, 2026
@thiagodeev thiagodeev force-pushed the thiagodeev/make-combine-layout-migration-mandatory branch from aac79fd to 6774432 Compare April 6, 2026 15:00
@thiagodeev thiagodeev merged commit 01f0479 into main Apr 6, 2026
53 of 61 checks passed
@thiagodeev thiagodeev deleted the thiagodeev/make-combine-layout-migration-mandatory branch April 6, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants