dkg: check disk writes #676
Conversation
Codecov Report
@@ Coverage Diff @@
## main #676 +/- ##
==========================================
+ Coverage 54.76% 54.92% +0.16%
==========================================
Files 101 101
Lines 9682 9755 +73
==========================================
+ Hits 5302 5358 +56
- Misses 3624 3626 +2
- Partials 756 771 +15
Continue to review full report at Codecov.
|
dkg/disk.go
Outdated
| if err != nil { | ||
| return errors.Wrap(err, "write deposit data") | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // fileExists returns error if keystores, cluster-lock and deposit-data already exists in given dataDir. | ||
| func fileExists(dataDir string, vals int) error { |
There was a problem hiding this comment.
suggest checking to see if you could write by actually writing a sample file since writing files can fail for multiple reasons, not just existing files...
There was a problem hiding this comment.
as per ticket:
The command should check for clashes (and all other disk write issues) at the start and fail with a proper message.
| } | ||
|
|
||
| if err := os.Remove(path.Join(dataDir, fmt.Sprintf("keystore-%d.txt", i))); err != nil { | ||
| return errors.Wrap(err, fmt.Sprintf("remove sample keystore-%d.txt", i)) |
There was a problem hiding this comment.
avoid sprintf in errors, rather use z.Str("file", filename)
dkg/disk.go
Outdated
| sigs[pk] = sig | ||
| } | ||
|
|
||
| if err := writeDepositData(sigs, "0x0000000000000000000000000000000000000000", "prater", dataDir); err != nil { |
There was a problem hiding this comment.
no const for this withdrawal addr avialble?
There was a problem hiding this comment.
can be replaced with testutil.RandomETHAddress()
| } | ||
|
|
||
| if err := writeDepositData(sigs, "0x0000000000000000000000000000000000000000", "prater", dataDir); err != nil { | ||
| return err |
There was a problem hiding this comment.
Are these error message human friendly? Will people understand what the problem is?
| func checkWrites(dataDir string, def cluster.Definition) error { | ||
| var shares []share | ||
| sigs := make(map[core.PubKey]*bls_sig.Signature) | ||
| for i := 0; i < def.NumValidators; i++ { |
There was a problem hiding this comment.
another option which is simpler is:
files := getSampleFiles(def.NumValidators)
for f in files {
err := os.WriteFile("sample", f, 0x444)
if err ! nil { "cannot write file", z.Str("file",f) }
err := os.Remove(f)
if err ! nil { "cannot delete file", z.Str("file",f) }
}
dB2510
changed the title
dB2510
added
the
merge when ready
Check disk writes by creating random sample files and then removing sample ones if successful.
category: bug
ticket: #584