search

balancer / balancer-v2-monorepo

Balancer V2 Monorepo
https://balancer.fi/
GNU General Public License v3.0
315 stars 382 forks source link

Was `pool-utils/contracts/oracle/QueryProcessor.sol` ever audited? #2570

Closed xenide closed 2 weeks ago

xenide commented 2 weeks ago

We're thinking of using the oracle code from here , but it seems like all oracle code infra was deleted in this commit.

I looked through all audit reports and none of them mention those oracle code. Can I confirm that:

  1. QueryProcessor.sol and associated files (such as Samples.sol and Buffer.sol) have never been audited?
    • if they've been, where are the audit reports?
  2. Therefore none of the pools deployed in production now have those oracle code?
xenide commented 2 weeks ago

paging @nventuro, could I trouble you take a quick look at this?

nventuro commented 2 weeks ago

Hm I don't recall exactly, but I don't think it was. None of the audits mention it, and I think it was developed and then removed in the time between deployment of v2 (April 2021) and deployment of the first set of stable pools (around Aug/Sept 2021).

From what I remember, we dropped it because we were unsure the oracles provided sufficient value to warrant the gas cost increases during normal operations, not due to correctness or security concerns.

xenide commented 2 weeks ago

hey @nventuro thanks for responding. In that case, then we're looking at cases of having deployed code without having gone through an audit (as seen in those two pools above).

Do you think this is what happened?

nventuro commented 2 weeks ago

Looking at the deployments, it seems like the only instance of a pool factory with a price oracle was the one you identified: https://github.com/balancer/balancer-deployments/tree/master/tasks/deprecated/20210727-meta-stable-pool

EndymionJkb commented 2 weeks ago

Looking at the deployments, it seems like the only instance of a pool factory with a price oracle was the one you identified: https://github.com/balancer/balancer-deployments/tree/master/tasks/deprecated/20210727-meta-stable-pool

Well, we also had the oracle weighted pool: https://github.com/balancer/balancer-deployments/tree/master/tasks/deprecated/20210418-weighted-pool (WeightedPool2Tokens).

My recollection is we were refactoring the pool code mid '22 (e.g., BPT protocol fees, composable pools), and deciding whether we wanted to retain the oracle functionality, which would likely have required maintaining multiple pool versions going forward. We wanted a single stable pool, and dropped the oracle functionality to keep it simple. The oracles just weren't used enough to be worth maintaining. And there was significant gas overhead if the oracle was turned on, as updating the oracle added storage writes to every operation.

I don't think the code was formally audited, but I seem to remember Certora at least looking at it. We certainly reviewed it extensively internally, and they were used in production without any issues (e.g., all the core, seeded weighted pools had oracles).

xenide commented 2 weeks ago

okay understood thanks for the insights @EndymionJkb @nventuro.

closing for now