Skip to content

Conversation

@rootulp
Copy link
Collaborator

@rootulp rootulp commented Nov 13, 2025

While working on #6177 I realized it is cumbersome to set the --v2-upgrade-height via ldflags.

This change reverts the deprecation for the --v2-upgrade-height. If a user passes the --v2-upgrade-height to the multiplexer, it forwards that to the embedded binaries.

Note for reviewers: we don't strictly need this change.

Testing

./scripts/single-node-all-upgrades.sh

...
Upgrade to version 6 complete!

Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

This change is breaking. Not sure if we should merge it now or wait for a major release 🤔

@rootulp rootulp changed the title chore: undeprecate v2 upgrade height chore!: undeprecate v2 upgrade height Nov 14, 2025
@rootulp
Copy link
Collaborator Author

rootulp commented Nov 14, 2025

Agreed it is breaking so will hold off on merging for now.

@rootulp rootulp added warn:api breaking item will be break an API and require a major bump WS: v7 labels Nov 14, 2025
@ninabarbakadze
Copy link
Member

This change is breaking. Not sure if we should merge it now or wait for a major release 🤔

afaiu we're just waiting to cut v6.x, right?

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 17, 2025

afaiu we're just waiting to cut v6.x, right?

Yes. IMO we shouldn't do that until we need to merge changes that are breaking (i.e. can't be merged to main). Likely sometime after v6 activation height on Mainnet.

github-merge-queue bot pushed a commit that referenced this pull request Nov 17, 2025
Closes #6177

The multiplexer was setting the v2 upgrade height to `0`. 

Short term: this PR adds a check to only append the
`--v2-upgrade-height` flag if it isn't set to 0.

Long term: remove the ldflags entirely in
#6197
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

warn:api breaking item will be break an API and require a major bump WS: v7

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants