-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
jwk #2: ensure jwk txns are expected in consensus #11855
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #11855 +/- ##
===========================================
- Coverage 71.2% 69.9% -1.3%
===========================================
Files 795 2181 +1386
Lines 182968 413637 +230669
===========================================
+ Hits 130275 289389 +159114
- Misses 52693 124248 +71555 ☔ View full report in Codecov by Sentry. |
@@ -73,6 +76,17 @@ pub fn storage(config: &SafetyRulesConfig) -> PersistentSafetyStorage { | |||
} | |||
} | |||
|
|||
pub fn load_consensus_key_from_secure_storage( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we already have something for DKG?
consensus/src/util/mod.rs
Outdated
|
||
pub fn is_vtxn_expected(features: &Features, vtxn: &ValidatorTransaction) -> bool { | ||
match vtxn { | ||
ValidatorTransaction::DummyTopic1(_) | ValidatorTransaction::DummyTopic2(_) => true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this shouldn't be true, it opens ddos attacks by validators? honestly I don't think it should even exist on non-testing path
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once the corresponding feature is enabled, DKG/JWK txn are similar?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DKG/JWK has verification logic that only valid ones can go through? the dummy txn execution just succeeds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can approve once dummy vtxn is marked test-only.
fixed |
…_jwk_txn_expected
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
Description
Test Plan