Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Optimize encoded-size of SignedCommitment #10409

Merged
merged 4 commits into from
Dec 3, 2021

Conversation

de-sh
Copy link
Contributor

@de-sh de-sh commented Dec 2, 2021

Fixes paritytech/grandpa-bridge-gadget#128

  • Creates a new, better compressed representation of SignedCommitment for BEEFY
  • Adds tests to check the same.

cc: @tomusdrw

@de-sh de-sh requested a review from adoerr as a code owner December 2, 2021 14:25
@de-sh de-sh changed the title Include code from paritytech/grandpa-bridge-gadget#186 Optimize encoded-size of SignedCommitment Dec 2, 2021
@adoerr adoerr added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit B0-silent Changes should not be mentioned in any release notes labels Dec 2, 2021
@adoerr adoerr requested a review from tomusdrw December 2, 2021 16:10
@adoerr
Copy link
Contributor

adoerr commented Dec 2, 2021

@de-sh thanks a lot for the PR! Could you please cargo fmt commitment.rs

Copy link
Contributor

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

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

lgtm! I'd just improve the docs a bit, cause they were unclear to me and rename signatures_len to validator_set_len.

primitives/beefy/src/commitment.rs Outdated Show resolved Hide resolved
primitives/beefy/src/commitment.rs Outdated Show resolved Hide resolved
primitives/beefy/src/commitment.rs Outdated Show resolved Hide resolved
primitives/beefy/src/commitment.rs Outdated Show resolved Hide resolved
@tomusdrw
Copy link
Contributor

tomusdrw commented Dec 3, 2021

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Checks failed for eae0152

@tomusdrw
Copy link
Contributor

tomusdrw commented Dec 3, 2021

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@paritytech-processbot paritytech-processbot bot merged commit e9ebf0c into paritytech:master Dec 3, 2021
@de-sh de-sh deleted the bitfield branch December 4, 2021 04:39
AurevoirXavier pushed a commit to darwinia-network/substrate that referenced this pull request Feb 7, 2022
* Include code from paritytech/grandpa-bridge-gadget#186

* Cargo fmt commitment.rs

* Make changes suggested by @tomusdrw

Co-authored-by: Tomasz Drwięga <tomasz@parity.io>
grishasobol pushed a commit to gear-tech/substrate that referenced this pull request Mar 28, 2022
* Include code from paritytech/grandpa-bridge-gadget#186

* Cargo fmt commitment.rs

* Make changes suggested by @tomusdrw

Co-authored-by: Tomasz Drwięga <tomasz@parity.io>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Include code from paritytech/grandpa-bridge-gadget#186

* Cargo fmt commitment.rs

* Make changes suggested by @tomusdrw

Co-authored-by: Tomasz Drwięga <tomasz@parity.io>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use bitfield in SignedCommitment for encoded-size optimisation.
3 participants