OEV gateway #1663
OEV gateway #1663
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| res.status(200).send(result!.data); | ||
| }; | ||
|
|
||
| app.post(oevGatewayPath, signOevDataRequestHandler); |
Check failure
Code scanning / CodeQL
Missing rate limiting
| }; | ||
|
|
||
| app.post(oevGatewayPath, signOevDataRequestHandler); | ||
| app.options(oevGatewayPath, signOevDataRequestHandler); |
Check failure
Code scanning / CodeQL
Missing rate limiting
packages/airnode-deployer/terraform/aws/templates/oevGw.yaml.tpl
Outdated
Show resolved
Hide resolved
|
|
||
| export type ProcessSignOevDataRequestBody = z.infer<typeof signOevDataBodySchema>; | ||
|
|
||
| export function dropInvalidBeacons(beacons: Beacon[]) { |
There was a problem hiding this comment.
I think the verification logic should be separated into separate function:
decodeBeacon- decodes encoded data and performs verifications, returns null in case something is wrong
Then you can do:
return beacons.map(decodeBeacon).filter(Boolean)
There was a problem hiding this comment.
Maybe call it maybeDecodeBeacon or something, but I think the above is good enough.
There was a problem hiding this comment.
What do you mean by verification logic? Checking the signature? Why? I actually tried to put as much "processing" of individual beacons into this function as possible so we don't need to iterate over them again and again.
There was a problem hiding this comment.
I personally don't mind, but would rename the function, as it's currently a bit misleading - it also modifies the returned (not dropped) beacons.
| return 0; | ||
| } | ||
|
|
||
| export function dropInconsistentBeacons(beacons: BeaconDecoded[]) { |
There was a problem hiding this comment.
| export function dropInconsistentBeacons(beacons: BeaconDecoded[]) { | |
| // TODO: Nit: rename to "chooseMaximumConsistentSet" or something | |
| export function dropInconsistentBeacons(beacons: BeaconDecoded[]) { | |
| const sortedBeacons = [...beacons].sort(beaconCompare); | |
| let maxConsistent: BeaconDecoded[] = []; | |
| // Check every beacon interval. The maximum consistent set of beacons for aggregation must have every element | |
| // deviating from the average by less than 1%. | |
| for (let start = 0; start < sortedBeacons.length; start++) { | |
| const tmp = [sortedBeacons[start]]; | |
| for (let end = start + 1; end < sortedBeacons.length; end++) { | |
| tmp.push(sortedBeacons[end]); | |
| // TODO: Extract to helper fn | |
| const avg = tmp.reduce((sum, beacon) => sum.add(beacon.decodedValue), ethers.BigNumber.from(0)).div(tmp.length); | |
| const maxDeviation = avg.div(100).mul(1); | |
| if ( | |
| // TODO: Can be extracted as well | |
| avg.sub(tmp.at(0)!.decodedValue).abs().lte(maxDeviation) && | |
| avg.sub(tmp.at(-1)!.decodedValue).abs().lte(maxDeviation) | |
| ) { | |
| if (tmp.length > maxConsistent.length) maxConsistent = tmp; | |
| } | |
| } | |
| } | |
| return maxConsistent; | |
| } |
There was a problem hiding this comment.
(btw. I didn't run/test the code in the suggestion so double check it).
There was a problem hiding this comment.
We also discussed about validator failing if there is OEV gateway without signed data one. But I don't remember the resolution.
EDIT: Found the discussion https://api3workspace.slack.com/archives/C02AYRX8D89/p1676288357122369?thread_ts=1676024035.659029&cid=C02AYRX8D89. We don't want validator to validate this.
| const currentTime = new Date(); | ||
|
|
||
| return beacons.reduce((result, beacon) => { | ||
| const goProcessing = goSync(() => { |
There was a problem hiding this comment.
nit:
| const goProcessing = goSync(() => { | |
| const goParseBeacon = goSync(() => { |
I dislike the name, but my suggestion is not great either... I like that all the verification is encapsulated in this single goSync though.
There was a problem hiding this comment.
What about goBeaconValidation? That sounds better to me.
| export async function signOevData( | ||
| requestBody: ProcessSignOevDataRequestBody, | ||
| validUpdateValues: ethers.BigNumber[] | ||
| ): Promise<[Error, null] | [null, SignOevDataResponse]> { |
There was a problem hiding this comment.
(nit) I've noticed it's by convention in this repo, but it's weird to me that the successful response has success property, but the error response doesn't.
|
|
||
| export type ProcessSignOevDataRequestBody = z.infer<typeof signOevDataBodySchema>; | ||
|
|
||
| export function dropInvalidBeacons(beacons: Beacon[]) { |
There was a problem hiding this comment.
I personally don't mind, but would rename the function, as it's currently a bit misleading - it also modifies the returned (not dropped) beacons.
| @@ -0,0 +1,191 @@ | |||
| openapi: "3.0.2" | |||
There was a problem hiding this comment.
It's not generated, it was written manually.
There was a problem hiding this comment.
👍 LGTM, but haven't tested on serverless.
Leaving that to @patriksimurka while implementing the related functionality for OEV relay.
|
After running the gateway on GCP I needed to change two more things:
|
Close #1579
I need to test the AWS and GCP gateways to see whether the OpenAPI templates are correct but other than that the code should be ready for a review.