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.

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

add some fail tips on nimbus_beacon_node deposits import #6270

Open
wants to merge 6 commits into
base: unstable
Choose a base branch
from

Conversation

xalexec
Copy link

@xalexec xalexec commented May 7, 2024

show some fail tips on import invalid keys

project https://github.com/PufferFinance/coral Puffer,
can generate validators file,but the keystore file not ending by .json
I use nimbus_beacon_node deposits import but nothing is shown,at first I thought this was correct.
But Validator not working.
I check the code and submit pr to improve it.

show some fail tips on import invalid keys
@tersec
Copy link
Contributor

tersec commented May 7, 2024

needs to target unstable branch

@xalexec xalexec changed the base branch from stable to unstable May 7, 2024 13:38
@xalexec
Copy link
Author

xalexec commented May 7, 2024

needs to target unstable branch

done.

@xalexec
Copy link
Author

xalexec commented May 30, 2024

some update for this pr?what can I do

@tersec
Copy link
Contributor

tersec commented May 30, 2024

Main points:

  • it adds a couple of flag variables to track state, which seems suboptimal, but I need to check closer if there's just no better way of handling this
  • the error/log messages are have less-than-perfect grammar/spelling/etc
  • not convinced of semantics of invalidFlag and/or message contents in case of multiple deposit directories, some of which are valid and some not

Will look more closely at these

Copy link

github-actions bot commented May 30, 2024

Unit Test Results

         9 files  ±  0    1 325 suites  +3   32m 41s ⏱️ + 3m 8s
  4 986 tests +  4    4 638 ✔️ +  4  348 💤 ±0  0 ±0 
20 814 runs   - 15  20 410 ✔️  - 15  404 💤 ±0  0 ±0 

Results for commit b642db9. ± Comparison against base commit 1b30dcc.

♻️ This comment has been updated with latest results.

Co-authored-by: tersec <tersec@users.noreply.github.com>
@xalexec
Copy link
Author

xalexec commented Jun 1, 2024

Reset the code,give up fatal exit

let importedFiles = walkDirRec(importedDir).toSeq
  if importedFiles.len == 0:
    fatal "No keystore file found at keystore path", key_path = importedDir
    quit 1
    
  for file in walkDirRec(importedDir):
    let filenameParts = splitFile(file)
    if toLowerAscii(filenameParts.ext) != ".json":
      warn "Keystore file must have a .json extension", file = file
      continue

How about this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants