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.

Sign up for GitHub

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

Jump to bottom

MESH-1680 bump libsecp256k1 #1

Merged
merged 1 commit into from
Sep 6, 2021
Merged

MESH-1680 bump libsecp256k1 #1

merged 1 commit into from
Sep 6, 2021

Conversation

Neopallium
Copy link

@Neopallium Neopallium commented Sep 2, 2021 •
edited
Loading

This only includes the libsecp256k1 changes from:
paritytech#9391

The libp2p changes were not included, because substrate had already upgraded from libp2p version 0.34 -> 0.37 before the above PR upgraded again to 0.39.

From the original PR:

  • RUSTSEC-2021-0076 bump libsecp256k1

libsecp256k1 allows overflowing signatures
https://rustsec.org/advisories/RUSTSEC-2021-0076

Changes were made to conform to libsecp256k1 version differences.

Closes paritytech#9356

  • parse_standard_slice() -> parse_overflowing_slice()

  • Added v2 host function for ecdsa_verify

  • Add feature tag over helpers

  • Added ecdsa_verify v2 to test runner

  • PR feedback

  • Spaces -> tabs
  • renamed two helper functions

  • Fixed imports after rebasing

  • Bump rest of libsecp256k1

  • Add version2 for ecdsa pubkey recovery

  • Update primitives/core/src/ecdsa.rs

  • Update primitives/core/src/ecdsa.rs

  • Update Cargo.lock

Co-authored-by: Bastian Köcher [email protected]

@trevor-crypto @bkchr @Neopallium
RUSTSEC-2021-0076 bump libsecp256k1 (paritytech#9391)
696d714 
* RUSTSEC-2021-0076 bump libsecp256k1

libsecp256k1 allows overflowing signatures
https://rustsec.org/advisories/RUSTSEC-2021-0076

Changes were made to conform to libsecp256k1 version differences.

Closes paritytech#9356

* parse_standard_slice() -> parse_overflowing_slice()

* Added v2 host function for ecdsa_verify

* Add feature tag over helpers

* Added ecdsa_verify v2 to test runner

* PR feedback

- Spaces -> tabs
- renamed two helper functions

* Fixed imports after rebasing

* Bump rest of libsecp256k1

* Add version2 for ecdsa pubkey recovery

* Update primitives/core/src/ecdsa.rs

* Update primitives/core/src/ecdsa.rs

* Update Cargo.lock

Co-authored-by: Bastian Köcher <[email protected]>
@Neopallium Neopallium mentioned this pull request Sep 2, 2021
Closed
@Neopallium
Copy link
Author

Polymesh PR PolymeshAssociation/Polymesh#1097
Uses this branch to test this PR.

Centril
Centril reviewed Sep 3, 2021
View reviewed changes
Cargo.lock
@@ -2984,7 +3007,7 @@ dependencies = [
"futures 0.3.12",
"futures-timer 3.0.2",
"lazy_static",
"libsecp256k1",
"libsecp256k1 0.3.5",
Copy link

Choose a reason for hiding this comment

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

Hmm, what happened here?

Copy link
Author

Choose a reason for hiding this comment

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

That is from libp2p. Upgrading libp2p will require a lot of changes, since there was a lot of API changes from 0.34 -> 0.39.

@Neopallium
Copy link
Author

FYI, I did a review of libp2p (version 0.34) to see if it was affected by RUSTSEC-2021-0076. libp2p uses libsecp256k1::Signature::parse_der for parsing signatures. parse_der correctly handled overflowing signatures.

Substrate was affected because they used libsecp256k1::Signature::parse_slice, which ignored overflows.

@adamdossa adamdossa merged commit 1fe1f43 into polymath-3.0.0 Sep 6, 2021
Neopallium pushed a commit that referenced this pull request May 5, 2022
@acatangiu @tomusdrw
Implement Lean BEEFY (paritytech#10882)
411d9bb 
Simplified BEEFY worker logic based on the invariant that GRANDPA
will always finalize 1st block of each new session, meaning BEEFY
worker is guaranteed to receive finality notification for the
BEEFY mandatory blocks.

Under these conditions the current design is as follows:
- session changes are detected based on BEEFY Digest present in
  BEEFY mandatory blocks,
- on each new session new `Rounds` of voting is created, with old
  rounds being dropped (for gossip rounds, last 3 are still alive
  so votes are still being gossiped),
- after processing finality for a block, the worker votes if
  a new voting target has become available as a result of said
  block finality processing,
- incoming votes as well as self-created votes are processed
  and signed commitments are created for completed BEEFY voting
  rounds,
- the worker votes if a new voting target becomes available
  once a round successfully completes.

On worker startup, the current validator set is retrieved from
the BEEFY pallet. If it is the genesis validator set, worker
starts voting right away considering Block #1 as session start.

Otherwise (not genesis), the worker will vote starting with
mandatory block of the next session.

Later on when we add the BEEFY initial-sync (catch-up) logic,
the worker will sync all past mandatory blocks Signed Commitments
and will be able to start voting right away.

BEEFY mandatory block is the block with header containing the BEEFY
`AuthoritiesChange` Digest, this block is guaranteed to be finalized
by GRANDPA.

This session-boundary block is signed by the ending-session's
validator set. Next blocks will be signed by the new session's
validator set. This behavior is consistent with what GRANDPA does
as well.

Also drop the limit N on active gossip rounds. In an adversarial
network, a bad actor could create and gossip N invalid votes with
round numbers larger than the current correct round number. This
would lead to votes for correct rounds to no longer be gossiped.

Add unit-tests for all components, including full voter consensus
tests.

Signed-off-by: Adrian Catangiu <[email protected]>
Co-authored-by: Tomasz Drwięga <[email protected]>
Co-authored-by: David Salami <Wizdave97>
Neopallium pushed a commit that referenced this pull request May 5, 2022
@agryaznov @athei @bkchr
Add frame_support::crypto::ecdsa::Public.to_eth_address() (k256-b…
a5cd385 
…ased) and use it in pallets (paritytech#11087)

* `ecdsa::Public::to_eth_address` + test, beefy-mmr `convert()` to use it, contracts Ext interface

* `seal_ecdsa_to_eth_address` all but benchmark done

* `seal_ecdsa_to_eth_address` + wasm test

* `seal_ecdsa_to_eth_address` + benchmark

* fixed dependencies

* Apply suggestions from code review

Co-authored-by: Alexander Theißen <[email protected]>

* fixes from review #1

* ecdsa::Public(*pk).to_eth_address() moved to frame_support and contracts to use it

* beefy-mmr to use newly added frame_support function for convertion

* a doc fix

* import fix

* benchmark fix-1 (still fails)

* benchmark fixed

* Apply suggestions from code review

Co-authored-by: Alexander Theißen <[email protected]>

* fixes on Alex T feedback

* to_eth_address() put into extension trait for sp-core::ecdsa::Public

* Update frame/support/src/crypto/ecdsa.rs

Co-authored-by: Alexander Theißen <[email protected]>

* Update frame/contracts/src/wasm/mod.rs

Co-authored-by: Alexander Theißen <[email protected]>

* fixes on issues pointed out in review

* benchmark errors fixed

* fmt fix

* EcdsaRecoverFailed err docs updated

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <[email protected]>

* make applied suggestions compile

* get rid of unwrap() in runtime

* Remove expect

Co-authored-by: Alexander Theißen <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: Bastian Köcher <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Centril Centril Centril approved these changes

@adamdossa adamdossa Awaiting requested review from adamdossa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Neopallium @Centril @adamdossa @trevor-crypto