Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fix: Proposal vote extensions' byte limit #585

Merged
merged 6 commits into from
Mar 3, 2025
Merged

Conversation

gitferry
Copy link
Member

@gitferry gitferry commented Feb 27, 2025

Closes https://github.com/babylonlabs-io/pm/issues/250. In the PR, PrepareProposal first adds the injected checkpoint tx and then add normal txs by order until byte limit is hit

@gitferry gitferry force-pushed the gai/fix-prepare-proposal branch from 26fb2cf to be9062d Compare February 27, 2025 15:53
@gitferry gitferry force-pushed the gai/fix-prepare-proposal branch from be9062d to d9c1096 Compare February 27, 2025 15:57
@gitferry gitferry marked this pull request as ready for review February 27, 2025 16:16
Copy link
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

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

utACK

Copy link
Collaborator

@KonradStaniec KonradStaniec left a comment

Choose a reason for hiding this comment

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

Only few minor nits, LGTM!

}

// SetCheckpointTx sets the tx used for checkpoint
func (t *PrepareProposalTxs) SetCheckpointTx(tx []byte) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe lets change name to SerOrReplaceCheckpointTx ?


// AddOtherTxs adds txs to the "other" tx category and return early
// if all the remaining bytes are used up
func (t *PrepareProposalTxs) AddOtherTxs(allTxs [][]byte) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe ReplaceOtherTxs ? Add suggests that it would extend exsiting ones but in the first line we are doing

t.OtherTxs = make([][]byte, 0, len(allTxs))

which mean all other txs will be cleared


// UpdateUsedBytes updates the used bytes field. This returns an error if the num used bytes
// exceeds the max byte limit.
func (t *PrepareProposalTxs) UpdateUsedBytes(
Copy link
Collaborator

Choose a reason for hiding this comment

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

it seems this should be private, as using this without corresponding transactions operations would leave PrepareProposalTxs struct in bad state

@gitferry gitferry merged commit 638d5ef into main Mar 3, 2025
24 checks passed
@gitferry gitferry deleted the gai/fix-prepare-proposal branch March 3, 2025 09:02
github-actions bot pushed a commit that referenced this pull request Mar 3, 2025
Closes babylonlabs-io/pm#250. In the PR,
PrepareProposal first adds the injected checkpoint tx and then add
normal txs by order until byte limit is hit

(cherry picked from commit 638d5ef)
Copy link

github-actions bot commented Mar 3, 2025

💚 All backports created successfully

Status Branch Result
release/v1.x

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

KonradStaniec pushed a commit that referenced this pull request Mar 3, 2025
# Backport

This will backport the following commits from `main` to `release/v1.x`:
- [fix: Proposal vote extensions' byte limit
(#585)](#585)

<!--- Backport version: 9.5.1 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

Co-authored-by: Cirrus Gai <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants