-
Notifications
You must be signed in to change notification settings - Fork 73
CBST2-02: Make proposer commitment signatures unique to modules #329
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
Changes from 65 commits
c68125d
d9979a2
97ef653
91eefe2
de09415
3aee63d
c07c717
699b7ec
7165f12
9438dae
12c020a
53cafc0
58c6117
45e581b
24a10c5
e36da54
843b110
e7c6d19
6117219
c0f591d
adbd34a
e3488b3
c3d7ec4
9ddad64
c62185e
dc73c62
6c3d967
6464638
0313f18
346eea4
7b20d2f
cf3f0b1
ca9f4a1
3eed526
aa6ad96
fc872ac
ca0c6e8
40d34aa
d537288
7afb763
09ac821
cf39d86
2431937
2e1198b
ccaf97d
145ebe8
bb0e023
71a7605
615774e
af6076d
cc13a6f
488547a
ef3d8f2
6fd4327
d9ef82f
83db727
8d08c11
64b49f2
8b65b1e
9967033
adb1cb8
1c3a07d
daf3147
fa36a34
d5641df
5da31bf
bfd72c5
1a0efec
ee282da
133447d
9f72aff
509dba8
2c507d7
d1ad9c9
ff716a0
de61066
db08d77
ce29c3a
b837e44
874e07d
3ad11a8
a75605e
d091a90
0313cc2
358b3cb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,11 +34,16 @@ pub const SIGNER_MODULE_NAME: &str = "signer"; | |
|
|
||
| /// Where the signer module should open the server | ||
| pub const SIGNER_ENDPOINT_ENV: &str = "CB_SIGNER_ENDPOINT"; | ||
| pub const SIGNER_PORT_DEFAULT: u16 = 20000; | ||
|
|
||
| // JWT authentication settings | ||
| /// Number of auth failures before rate limiting the client | ||
| pub const SIGNER_JWT_AUTH_FAIL_LIMIT_ENV: &str = "CB_SIGNER_JWT_AUTH_FAIL_LIMIT"; | ||
| pub const SIGNER_JWT_AUTH_FAIL_LIMIT_DEFAULT: u32 = 3; | ||
|
|
||
| /// How long to rate limit the client after auth failures | ||
| pub const SIGNER_JWT_AUTH_FAIL_TIMEOUT_SECONDS_ENV: &str = | ||
| "CB_SIGNER_JWT_AUTH_FAIL_TIMEOUT_SECONDS"; | ||
| pub const SIGNER_JWT_AUTH_FAIL_TIMEOUT_SECONDS_DEFAULT: u32 = 5 * 60; | ||
|
Comment on lines
+41
to
+46
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this supposed to be in this PR or does it need to be rebased?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was some incidental cleanup of #310 that landed in here after it was merged. There are a few little non-functional changes related to it. For cleanliness I can pull all of those out and make a separate PR for them if you'd like, no problem.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep let's do that to keep the PR clean
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in PR #346 which has been merged into this now. |
||
|
|
||
| /// Comma separated list module_id=jwt_secret | ||
| pub const JWTS_ENV: &str = "CB_JWTS"; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| use std::collections::HashMap; | ||
|
|
||
| use alloy::primitives::B256; | ||
| use eyre::{ContextCompat, Result}; | ||
| use serde::{de::DeserializeOwned, Deserialize, Serialize}; | ||
| use toml::Table; | ||
|
|
@@ -37,6 +38,8 @@ pub struct StaticModuleConfig { | |
| /// Type of the module | ||
| #[serde(rename = "type")] | ||
| pub kind: ModuleKind, | ||
| /// Signing ID for the module to use when requesting signatures | ||
| pub signing_id: Option<B256>, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why make it optional?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. also worth creating a type alias for clarity
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
So this was a concession I had to make in order to support the other use cases where the signature generation pipeline is used with
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Even if we want to still make it optional internally, it should not be optional in the module configuration. After this PR, modules need to have this set
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, that makes sense. Fixed in 509dba8.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with making it required for commit modules, but I think for PBS Events modules it's useless as they won't sign anything. With that said, it's not a tedious config to set, so I wouldn't mind to leave it required in favor of a cleaner codebase
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #338 is going to remove PBS events modules anyway, so assuming that goes in soon-ish then this may end up being OBE? |
||
| } | ||
|
|
||
| /// Runtime config to start a module | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.