Conversation
cason
reviewed
Apr 10, 2024
cason
left a comment
There was a problem hiding this comment.
Left some general comments.
Not sure if the solution for having this key optional, depending on the compiling flags, is ideal.
types/block.go
Outdated
| // Commit sig size is made up of 96 bytes for the signature, 20 bytes for the address, | ||
| // 1 byte for the flag and 14 bytes for the timestamp. | ||
| MaxCommitSigBytes int64 = 109 | ||
| MaxCommitSigBytes int64 = 141 |
There was a problem hiding this comment.
I am afraid the math here is wrong: 96 + 20 + 1 + 14 = 131.
Can we somehow test this?
Collaborator
Author
There was a problem hiding this comment.
The math was wrong before, too. I don't know why exactly.
types/block.go
Outdated
| MaxCommitSigBytes int64 = 141 | ||
|
|
||
| // protoEncodingOverhead represents the overhead in bytes when encoding a protocol buffer message. | ||
| protoEncodingOverhead int64 = 2 |
Collaborator
Author
There was a problem hiding this comment.
It's probably because it's an implementation detail.
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
| github.com/cometbft/cometbft-db v0.11.1-0.20240407063702-fa37a805b0b4 | ||
| github.com/cometbft/cometbft-load-test v0.1.0 | ||
| github.com/cometbft/cometbft/api v1.0.0-alpha.2 | ||
| github.com/cosmos/crypto v0.0.0-20240309083813-82ed2537802e |
sergio-mena
reviewed
Apr 17, 2024
sergio-mena
reviewed
Apr 17, 2024
Collaborator
|
I added a few comments, some of them important. Sorry for arriving late to this review |
This was referenced Apr 17, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Apr 18, 2024
- pin cometbft-db-testing image version - extract const in a separate file - return err in GenPrivKey and NewPrivateKeyFromBytes Follow-up to #2765 --- #### PR checklist - [x] Tests written/updated - [ ] ~~Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)~~ - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Sergio Mena <sergio@informal.systems>
mergify bot
pushed a commit
that referenced
this pull request
Apr 18, 2024
- pin cometbft-db-testing image version - extract const in a separate file - return err in GenPrivKey and NewPrivateKeyFromBytes Follow-up to #2765 --- #### PR checklist - [x] Tests written/updated - [ ] ~~Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)~~ - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Sergio Mena <sergio@informal.systems> (cherry picked from commit d27732a)
4 tasks
melekes
added a commit
that referenced
this pull request
Apr 18, 2024
… (#2842) - pin cometbft-db-testing image version - extract const in a separate file - return err in GenPrivKey and NewPrivateKeyFromBytes Follow-up to #2765 --- #### PR checklist - [x] Tests written/updated - [ ] ~~Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)~~ - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec <hr>This is an automatic backport of pull request #2832 done by [Mergify](https://mergify.com). Co-authored-by: Anton Kaliaev <anton.kalyaev@gmail.com>
4 tasks
cason
reviewed
May 8, 2024
| github.com/goccmack/goutil v1.2.3 | ||
| github.com/gofrs/uuid v4.4.0+incompatible | ||
| github.com/google/uuid v1.6.0 | ||
| github.com/minio/sha256-simd v1.0.1 |
There was a problem hiding this comment.
Why using this version instead of the standard one?
aljo242
pushed a commit
that referenced
this pull request
Sep 4, 2025
[curve](https://github.com/cosmos/crypto/tree/marko/bls12381) based on [blst](https://github.com/supranational/blst). The `bls12381` build tag was added. Also, note that BLS requires `cgo`. Co-authored by: @tac0turtle --- - [x] Tests written/updated - [x] Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog) - [ ] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Marko Baricevic <markobaricevic3778@gmail.com> Co-authored-by: Marko <marko@baricevic.me>
aljo242
pushed a commit
that referenced
this pull request
Sep 4, 2025
- pin cometbft-db-testing image version - extract const in a separate file - return err in GenPrivKey and NewPrivateKeyFromBytes Follow-up to #2765 --- - [x] Tests written/updated - [ ] ~~Changelog entry added in `.changelog` (we use [unclog](https://github.com/informalsystems/unclog) to manage our changelog)~~ - [x] Updated relevant documentation (`docs/` or `spec/`) and code comments - [x] Title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/) spec --------- Co-authored-by: Sergio Mena <sergio@informal.systems>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
curve based on blst.
The
bls12381build tag was added. Also, note that BLS requirescgo.Co-authored by: @tac0turtle
PR checklist
.changelog(we use unclog to manage our changelog)docs/orspec/) and code comments