Clarify usage of Attestation vs SingleAttestation#492
Clarify usage of Attestation vs SingleAttestation#492rolfyone merged 1 commit intoethereum:masterfrom
Conversation
| example: "phase0" | ||
| data: | ||
| anyOf: | ||
| oneOf: |
There was a problem hiding this comment.
I don't really see the difference between any and one of, it's mostly semantics but the usage of oneOf is problematic as it can cause an invalid oapi spec, see #421 and related issue #393.
The issue is that if you pass a value that matches multiple of the schemas then it fails when oneOf is used while it passes with anyOf. This causes false positives when running api spec tests and some schema parsers don't accept invalid oapi specs so we had to patch it locally before running our tests previously.
Now in this specific case I think it's not a issue since the both types are different enough and phase0.Attestation will not fit the schema of SingleAttestation (and vice versa) but this is something to keep in mind.
There was a problem hiding this comment.
To me this is just documentation, so if a change to oneOf is a problem I'm fine with leaving it as anyOf. neither really enforces at slot X , type Y must be used.
There was a problem hiding this comment.
The main part of this is the clarifying text, if people want to keep it as anyOf then fair enough, although as mentioned there isn't an overlap situation here so anyOf should work.
There was a problem hiding this comment.
I tested usage of oneOf in our spec tests and as expected it does not cause issues due to the difference in schemas, I am ok with keeping this change as it's seem to help in terms of semantics here
nflaig
left a comment
nflaig
left a comment
There was a problem hiding this comment.
LGTM - but might wanna rephrase the PR title a little
rolfyone
changed the title
3 participants
Add clarifying text as to when to supply
Attestationand whenSingleAttestation