feat: Fault Testing Support #47
Conversation
…ociask--feat-fault-testing-support
…ociask--feat-fault-testing-support
| if err != nil { | ||
| return nil, err | ||
| } | ||
| } else { | ||
| log.Info("Using EigenDA backend") | ||
| } | ||
|
|
||
| if vCfg.Verify { |
There was a problem hiding this comment.
nix, maybe move the code block up
| ct, err := ReadCommitmentMode(r) | ||
| if err != nil { | ||
| svr.WriteBadRequest(w, invalidCommitmentMode) | ||
| return err | ||
| } | ||
|
|
||
| actor := ReadActor(r) |
There was a problem hiding this comment.
I assume we only want memstore to behave like a malicious actor. Can we not touch the code path from other storage type.
There was a problem hiding this comment.
we can but that'd increase the attack surface for critical system flows that likely shouldn't ever have modifiable return behaviors (i.e, comms with mainnet). we only should ever use this feature for testing sequencer behaviors when doing QA / experimentation
There was a problem hiding this comment.
So can we move actor inside memstore go files, so it no longer appears in the top level codeabse
There was a problem hiding this comment.
how would we do that when it needs to be read from the URL params?
There was a problem hiding this comment.
I guess my concern is that we only intend memstore to have the ability to behave maliciously, then we should localize the change to the memstore itsel. Because memstore is not intended for production use in any cases. Then I feel it is relatively safe.
My understanding of of the code is that, it uses the context to determine proxy behavior. I worry it touches unnecesary part of code, making it bit harder to maintain. I worry we might accidentally inject some malicious behavior in the production path. Just my thoughts, I would invite @jianoaix to take a look.
There was a problem hiding this comment.
Is this server for production? Is production server taking the "actor" param from http request, based on which to decide the malicious behavior which is implemented in the store?
There was a problem hiding this comment.
it is but these malicious behavior patterns are only expressed within the memstore implementation which will never be used in production environments.
| @@ -60,6 +61,10 @@ func (c *client) Health() error { | |||
| func (c *client) GetData(ctx context.Context, comm []byte) ([]byte, error) { | |||
| url := fmt.Sprintf("%s/get/0x%x?commitment_mode=simple", c.cfg.URL, comm) | |||
|
|
|||
| if c.cfg.Actor != "" { | |||
There was a problem hiding this comment.
One high level comment, the eigenda-proxy should be configured to work correctly most of time. Is it reasonable to make Actor a lower layer concept subject to a specific boundary.
| G1PathFlagName = "eigenda-g1-path" | ||
| G2TauFlagName = "eigenda-g2-tau-path" | ||
| CachePathFlagName = "eigenda-cache-path" | ||
| MaxBlobLengthFlagName = "eigenda-max-blob-length" |
There was a problem hiding this comment.
Can we use "length" and "size" consistently? "length" is number of symbols from the field (32 bytes per symbol), and "size" is number of bytes
| } | ||
|
|
||
| func LoadFaultConfig(path string) (*FaultConfig, error) { | ||
| println(fmt.Sprintf("Loading config from %s", path)) |
There was a problem hiding this comment.
Remove debug info here and below?
| ct, err := ReadCommitmentMode(r) | ||
| if err != nil { | ||
| svr.WriteBadRequest(w, invalidCommitmentMode) | ||
| return err | ||
| } | ||
|
|
||
| actor := ReadActor(r) |
There was a problem hiding this comment.
Is this server for production? Is production server taking the "actor" param from http request, based on which to decide the malicious behavior which is implemented in the store?
Fault Testing Support
Fixes Issue
Fixes #21
Changes proposed
readme.mdwith fault mode info and updated soft confirmation docsScreenshots (Optional)
Note to reviewers