QA Report

[code423n4]

Summary

Low Risk Issues

	Issue	Contexts
LOW‑1	Missing parameter validation in constructor	4

Total: 4 contexts over 1 issues

Non-critical Issues

	Issue	Contexts
NC‑1	Use a more recent version of Solidity	1
NC‑2	Event Is Missing Indexed Fields	4
NC‑3	Implementation contract may not be initialized	1
NC‑4	Unused event may be unused code or indicative of missed emit/logic	1
NC‑5	Non-usage of specific imports	8
NC‑6	Lines are too long	2

Total: 17 contexts over 6 issues

Low Risk Issues

[LOW‑1] Missing parameter validation in constructor

Some parameters of constructors are not checked for invalid values.

Proof Of Concept

132: constructor(
133:    address _votingEscrow,
134:    address _delegationBoost,
135:    address _chestAddress,
136:    uint256 _minTargetVotes
137: ) {

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L132-L137

Recommended Mitigation Steps

Validate the parameters.

Non Critical Issues

[NC‑1] Use a more recent version of Solidity

Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(,) Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions

Proof Of Concept

pragma solidity 0.8.10;

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L2

Recommended Mitigation Steps

Consider updating to a more recent solidity version.

[NC‑2] Event Is Missing Indexed Fields

Index event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields).

Each event should use three indexed fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.

Proof Of Concept

event NewPledge(
        address creator,
        address receiver,
        address rewardToken,
        uint256 targetVotes,
        uint256 rewardPerVote,
        uint256 endTimestamp
    );

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L85

event ChestUpdated(address oldChest, address newChest);

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L115

event PlatformFeeUpdated(uint256 oldfee, uint256 newFee);

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L117

event MinTargetUpdated(uint256 oldMinTarget, uint256 newMinTargetVotes);

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L119

[NC‑3] Implementation contract may not be initialized

OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5

Proof Of Concept

132: constructor(
133:    address _votingEscrow,
134:    address _delegationBoost,
135:    address _chestAddress,
136:    uint256 _minTargetVotes
137: ) {

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L132-L137

[NC‑4] Unused event may be unused code or indicative of missed emit/logic

Events that are declared but not used may be indicative of unused declarations where it makes sense to remove them for better readability/maintainability/auditability, or worse indicative of a missing emit which is bad for monitoring or missing logic that would have emitted that event.

Proof Of Concept

96: event IncreasePledgeTargetVotes

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L96

Recommended Mitigation Steps

Add emit or remove event declaration.

[NC‑5] Non-usage of specific imports

The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files

Proof Of Concept

4: import "./oz/interfaces/IERC20.sol";
5: import "./oz/libraries/SafeERC20.sol";
6: import "./utils/Owner.sol";
7: import "./oz/utils/Pausable.sol";
8: import "./oz/utils/ReentrancyGuard.sol";
9: import "./interfaces/IVotingEscrow.sol";
10: import "./interfaces/IBoostV2.sol";
11: import "./utils/Errors.sol";

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L4-L11

#### <ins>Recommended Mitigation Steps</ins>

Use specific imports syntax per solidity docs recommendation.

### <a href="#Summary">[NC&#x2011;6]</a><a name="NC&#x2011;6"> Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length
Reference: https://docs.soliditylang.org/en/v0.8.10/style-guide.html#maximum-line-length

#### <ins>Proof Of Concept</ins>

239: // Check that the user has enough boost delegation available & set the correct allowance to this contract

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L239

261: // based on the Boost bias & the Boost duration, to take in account that the delegated amount decreases

https://github.com/code-423n4/2022-10-paladin/tree/main/contracts/WardenPledge.sol#L261

[kirk-baird]

NC-3 is not applicable here as it is not a proxy contract that inherits Initializeable

[c4-judge]

kirk-baird marked the issue as grade-b