search

Byte-Masons / Reliquary

50 stars 20 forks source link

Guardian Audit Review #22

Closed GuardianAudits closed 2 years ago

GuardianAudits commented 2 years ago

Guardian began an analysis of Reliquary on May 18th at commit 11a2e1343004d7571e283dd47b3bae4a03be8696, since then numerous commits/changes have been made so we've decided to cut our analysis short.

Here are the findings from our first pass: https://github.com/GuardianAudits/Audits/blob/main/Reliquary_Audit.pdf

If there's likely to be a code freeze/a commit you'd like us to focus on, we're more than happy to pick back up and complete a full analysis.

If there are any questions about our findings, we're more than happy to discuss them in this issue.

Zokunei commented 2 years ago

Thank you for your high-quality audit, despite not being able to devote full time to it. I apologize for the lack of communication regarding ongoing development. Our team has reviewed the contracts and are ready to implement a code freeze after the latest commit, e1b10ca928bbdb63dc9a7fa1132837650ceb44f5.

Regarding your previous findings:

Global-1

It is true that some circumstances could result in users not being able to claim rewards. The first way (not enough OATH available in the Reliquary contract) was easy enough to fix because a position's rewardCredit already keeps track of how much OATH is owed. The other centralization risks will be addressed by giving OPERATOR status to multi-sig/governance.

RLQ-1

This issue cannot be solved in the manner suggested without significant tradeoffs. Due to gas limitations we cannot loop through every position on _updatePosition to determine the exact amount of tokens that should be counted as sitting in each maturity level. This means the best way for us to get an accurate estimation is to incentivize users to each call updatePosition on their own position once it has matured enough. Note that updatePosition can also be called permissionlessly if necessary to provide better calculations.

RLQ-2

This sounds like it is describing intended behavior.

RLQ-3

We do not intend to support burn-on-transfer tokens.

RLQ-4

We have implemented the suggested changes.

RLQ-5

We have implemented the suggested changes.

RLQ-6

We have implemented the suggested changes.

DESC-1

We will reexamine this upgradable component if we ever intend to support tokens that will cause any of the issues described.

REW-6

This is intended behavior.