fix: --p2p-secret-key silently reverts to default location if file is not found (#2498)#2508
fix: --p2p-secret-key silently reverts to default location if file is not found (#2498)#2508mightypenguin wants to merge 2 commits intoparadigmxyz:mainfrom
Conversation
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #2508 +/- ##
==========================================
- Coverage 71.68% 71.63% -0.06%
==========================================
Files 489 489
Lines 61020 61088 +68
==========================================
+ Hits 43744 43761 +17
- Misses 17276 17327 +51
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Not sure if we want to make changes to other places that use secret_key.rs? |
mattsse
left a comment
mattsse
left a comment
There was a problem hiding this comment.
would like to see some tests for this, unclear how this fixes it because it looks like the function still behaves exactly as before.
| let mut secret_key_path = secret_key_path.as_path(); | ||
| if secret_key_path.to_str() == Some("") { | ||
| secret_key_path = default_secret_key_path.as_path(); | ||
| } |
There was a problem hiding this comment.
what's the reasoning for this here?
There was a problem hiding this comment.
The current code always returns the default secret_key_path and does NOT use the --p2p-secret-key parameter if set.
My changes use "p2p-secret-key" if specified, else, use default value.
There was a problem hiding this comment.
sorry, perhaps I misunderstood
so the actual bug here is that the user input is ignored?
silently reverts to default location if file is not found
There was a problem hiding this comment.
how exactly does your change fix
Setup Reth so that the default location for the p2p secret key is not accessible (e.g. systemd ProtectHome=True).
Set --p2p-secret-key to a non existent file.
This results in the following very confusing failure:
There was a problem hiding this comment.
Problems Address here:
-
User input is ignored.
This is a new problem introduced the the last couple days that I ran across and tried to fix here.
Blame Commit:
feat: use chain-specific data dirs (feat: use chain-specific data dirs #2495)
ecc7ae9 -
Original problem
The --p2p-path-key set by user.
If the file does not exist, then the default_path is silently used/created without erroring.
This PR Addresses 2 cases:
- If p2p-secret-key IS set, try to load it, if it doesn't exist, create it.
- If p2p-secret-key is NOT set, use default path, if it doesn't exist create it.
I believe the reported issue is OBE now. |
3 participants
Resolves #2498