QA Report

[code423n4]

List of contents

Low

• Performance fee may be set above maxPerformanceFee
• Funding.sol accepts invalid discount limits
• Funding.sol accepts invalid price bounds
• Funding.sweep sweeps all citadel rather than just excess

Non-Critical

• Incorrect comment on Funding.sweep

Low

Performance fee may be set above maxPerformanceFee

`StakedCitadel defines a max performance fee which may be set at any value up to 30%

https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/StakedCitadel.sol#L533-L542

This is broken up into two components, performance fees paid to the strategist and performance fees paid to governance

https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/StakedCitadel.sol#L618-L636
https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/StakedCitadel.sol#L638-L656

When setting both these components of the performance fee, it's checked that they don't exceed the cap individually but not that they don't exceed the cap together. The real performance fee cap is then 2 * maxPerformanceFee.

Funding.sol accepts invalid discount limits

Funding.sol checks that the provided max and min discount factors don't exceed a maximum discount but does not check that _minDiscount <= _maxDiscount.

https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/Funding.sol#L356-L366

This results in the discountManager being unable to provide a value which would satisfy these constraints in setDiscount().

https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/Funding.sol#L265-L276

Funding.sol accepts invalid price bounds

Funding.sol accepts minimum and maximum price bounds without any validation that the minimum is less than the maximum.

https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/Funding.sol#L397-L406

It's then possible to put the contract in a state such that the price may not be updated until the bounds are fixed.

https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/Funding.sol#L427-L430

Funding.sweep sweeps all citadel rather than just excess

The natspec of Funding.sweep suggests that calling sweep(citadel) would be safe in that it would leave enough CTDL to satisfy any outstanding claims. The function implementation however has no special logic for CTDL.
https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/Funding.sol#L310-L312

Non-Critical

Incorrect comment on Funding.sweep

Here we say in a comment that asset builds up on Funding until manually swept to the treasury.
https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/Funding.sol#L333

This is not true as on each sale it is sent directly to saleRecipient.
https://github.com/code-423n4/2022-04-badger-citadel/blob/dab143a990a9c355578fbb15cd3c884614e33f42/src/Funding.sol#L180