Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QA Report #36

Open
c4-bot-6 opened this issue Sep 12, 2024 · 4 comments
Open

QA Report #36

c4-bot-6 opened this issue Sep 12, 2024 · 4 comments
Labels
2nd place bug Something isn't working edited-by-warden grade-a Q-16 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality

Comments

@c4-bot-6
Copy link
Contributor

See the markdown file with the details of this report here.

@c4-bot-6 c4-bot-6 added bug Something isn't working QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax labels Sep 12, 2024
c4-bot-4 added a commit that referenced this issue Sep 12, 2024
c4-bot-5 added a commit that referenced this issue Sep 12, 2024
c4-bot-7 added a commit that referenced this issue Sep 13, 2024
c4-bot-6 added a commit that referenced this issue Sep 13, 2024
@af-afk
Copy link

af-afk commented Sep 18, 2024

We'll fix https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-01-inconsistent-overflow-handling-of-sqrt-price-may-cause-unexpected-swap-behaviors.

We don't agree with https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-02-fusdc-rewards-will-likely-be-locked-in-amms-proxy, what happens in practice is that the Fluidity worker has a special event decoding feature that knows where to send rewards. In practice, we have a "amm rewards server" that does this work.

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-03-risk-of-mint_position_b_c5_b086_d-spamming-with-0-liquidity-for-profits We don't agree this is likely (or feasible).

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-04-seawateramm-proxy-has-immutable-facet-function-signatures-which-might-cause-conflict-in-future-upgrades We don't agree with this, selector conflict is not likely with the immutable functions to be an issue (especially since we can mine new ones).

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-05-facet-contracts-cannot-be-upgraded-independently-from-other-facet-contracts We acknowledge this can be wasteful and error-prone, but in practice we believe it's better to have a holistic view of the upgrades, because the code includes embedded paths that may need to be reproduced in each of the facets.

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-06-swap_2-allows-0-amount-swap-to-pass We'll fix this!

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-07-change-lower-into-upper We'll fix this.

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-08-position-can-be-updated-in-the-wrong-pool-due-to-insufficient-checks We'll fix this!

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-09-pools-that-have-not-been-created-can-be-enabled-users-can-interact-with-a-pool-in-an-uninitialized-state We'll fix this!

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-10--unnecessary-implementation--send_amounts_from_sender We won't fix this. The migrations facet is not intended for end users, and it's more "hacky" than the other code. It's useful in a migration of amounts to a new contract (in a testnet environment for example). Though we acknowledge the issue.

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-12-ownershipnftssol-is-missing-erc165supportinterface-no-incompliance-with-eip-721 Compliance with the spec isn't a goal, but we'll fix this.

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-13-swap_internal-has-invalid-check-on-amount_0-and-amount_1 We'll fix this.

https://github.com/code-423n4/2024-08-superposition-findings/blob/main/data/oakcobalt-Q.md#low-14-nft-position-minted-without-checking-without-checking-user-implements-onerc721received We'll fix this.

@af-afk af-afk added sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue labels Sep 18, 2024
@liveactionllama liveactionllama added the sufficient quality report This report is of sufficient quality label Sep 18, 2024
@liveactionllama
Copy link
Contributor

Adding the sufficient quality label on behalf of the validator.

@c4-judge
Copy link
Contributor

alex-ppg marked the issue as grade-a

@thebrittfactor
Copy link

For awarding purposes, C4 staff have marked as 2nd place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2nd place bug Something isn't working edited-by-warden grade-a Q-16 QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

7 participants