Golom contest - pfapostol's results

An NFT marketplace that offers the lowest industry fee, a publicly available order-book along with analytical tools.

General Information

Platform: Code4rena

Start Date: 26/07/2022

Pot Size: $75,000 USDC

Total HM: 29

Participants: 179

Period: 6 days

Judge: LSDan

Total Solo HM: 6

Id: 148

League: ETH

Golom

Findings Distribution

Researcher Performance

Rank: 164/179

Findings: 1

Award: $21.32

🌟 Selected for report: 0

🚀 Solo Findings: 0

Estimated gas savings may be less than actual because code coverage is too low (I've written some tests for VoteEscrow, so deployment gas should be accurate, but methods are not covered.)

  1. The result of external function calls should be cached rather than re-calling the function Gas Saved: 358451
  • 2022-07-golom/contracts/rewards/RewardDistributor.sol:100,112-114,197,198,202,203,239,240,244,245
100        if (rewardToken.totalSupply() > 1000000000 * 10**18) {
...
112            uint256 tokenToEmit = (dailyEmission * (rewardToken.totalSupply() - rewardToken.balanceOf(address(ve)))) /
113                rewardToken.totalSupply();
114            uint256 stakerReward = (tokenToEmit * rewardToken.balanceOf(address(ve))) / rewardToken.totalSupply();
...
197                        (rewardStaker[epochs[index]] * ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[epochs[index]])) /
198                        ve.totalSupplyAt(epochBeginTime[epochs[index]]);
...
202                            ve.balanceOfAtNFT(tokenids[tindex], epochBeginTime[epochs[index]])) /
203                        ve.totalSupplyAt(epochBeginTime[epochs[index]]);
...
239                        (rewardStaker[index] * ve.balanceOfAtNFT(tokenid, epochBeginTime[index])) /
240                        ve.totalSupplyAt(epochBeginTime[index]);
...
244                            ve.balanceOfAtNFT(tokenid, epochBeginTime[index])) /
245                        ve.totalSupplyAt(epochBeginTime[index]);

2022-07-golom/contracts/rewards/RewardDistributor.sol:100

2022-07-golom/contracts/rewards/RewardDistributor.sol:112-114

2022-07-golom/contracts/rewards/RewardDistributor.sol:197-198

2022-07-golom/contracts/rewards/RewardDistributor.sol:202-203

2022-07-golom/contracts/rewards/RewardDistributor.sol:239-240

2022-07-golom/contracts/rewards/RewardDistributor.sol:244-245

  1. Use custom errors rather than revert()/require() strings to save gas (44 instances) Gas saved: 226900

    • 2022-07-golom/contracts/governance/GolomToken.sol:24,43,51,69
24          require(msg.sender == minter, 'GolomToken: only reward distributor can enable');
...
43          require(!isAirdropMinted, 'already minted');
...
51          require(!isGenesisRewardMinted, 'already minted');
...
69          require(minterEnableDate <= block.timestamp, 'GolomToken: wait for timelock');

2022-07-golom/contracts/governance/GolomToken.sol:24

2022-07-golom/contracts/governance/GolomToken.sol:43

2022-07-golom/contracts/governance/GolomToken.sol:51

2022-07-golom/contracts/governance/GolomToken.sol:69

  • 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:72-73,99,130,186,211,239
72          require(ownerOf(tokenId) == msg.sender, 'VEDelegation: Not allowed');
73          require(this.balanceOfNFT(tokenId) >= MIN_VOTING_POWER_REQUIRED, 'VEDelegation: Need more voting power');
...
99          require(_delegatedTokenIds.length < 500, 'VVDelegation: Cannot stake more');
...
130         require(blockNumber < block.number, 'VEDelegation: not yet determined');
...
186         require(blockNumber < block.number, 'VEDelegation: not yet determined');
...
211         require(ownerOf(tokenId) == msg.sender, 'VEDelegation: Not allowed');
...
239         require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:72-73

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:99

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:130

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:186

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:211

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:239

  • 2022-07-golom/contracts/rewards/RewardDistributor.sol:173,181,184-185,220,292,309
173         require(address(ve) != address(0), ' VE not added yet');
...
181         require(tokenowner == ve.ownerOf(tokenids[tindex]), 'Can only claim for a single Address together');
...
184         require(epochs[index] < epoch, 'cant claim for future epochs');
185         require(claimed[tokenids[tindex]][epochs[index]] == 0, 'cant claim if already claimed');
...
220         require(address(ve) != address(0), ' VE not added yet');
...
292         require(traderEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');
...
309         require(voteEscrowEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');

2022-07-golom/contracts/rewards/RewardDistributor.sol:173

2022-07-golom/contracts/rewards/RewardDistributor.sol:181

2022-07-golom/contracts/rewards/RewardDistributor.sol:184-185

2022-07-golom/contracts/rewards/RewardDistributor.sol:220

2022-07-golom/contracts/rewards/RewardDistributor.sol:292

2022-07-golom/contracts/rewards/RewardDistributor.sol:309

  • 2022-07-golom/contracts/rewards/GolomTrader.sol:177,211-214,217,222,226-227,236,301,361,457
177         require(signaturesigner == o.signer, 'invalid signature');
...
211         require(
212          o.totalAmt >= o.exchange.paymentAmt + o.prePayment.paymentAmt + o.refererrAmt + (o.totalAmt * 50) / 10000,
213          'amt not matching'
214      );
...
217         require(msg.value >= o.totalAmt * amount + p.paymentAmt, 'mgmtm');
...
222         require(o.orderType == 0, 'invalid orderType');
...
226         require(status == 3, 'order not valid');
227         require(amountRemaining >= amount, 'order already filled');
...
236         require(amount == 1, 'only 1 erc721 at 1 time');
...
301         require(amount == 1, 'only 1 erc721 at 1 time');
...
361         require(amount == 1, 'only 1 erc721 at 1 time');
...
457         require(distributorEnableDate <= block.timestamp, 'not allowed');

2022-07-golom/contracts/rewards/GolomTrader.sol:177

2022-07-golom/contracts/rewards/GolomTrader.sol:211-214

2022-07-golom/contracts/rewards/GolomTrader.sol:217

2022-07-golom/contracts/rewards/GolomTrader.sol:222

2022-07-golom/contracts/rewards/GolomTrader.sol:226-227

2022-07-golom/contracts/rewards/GolomTrader.sol:236

2022-07-golom/contracts/rewards/GolomTrader.sol:301

2022-07-golom/contracts/rewards/GolomTrader.sol:361

2022-07-golom/contracts/rewards/GolomTrader.sol:457

  • 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:538,894,928-929,945-946,982-983,996-999,1008,1011,1082,1227
538         require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');
...
894         require(attachments[_from] == 0 && !voted[_from], 'attached');
...
928         require(_locked.amount > 0, 'No existing lock found');
929         require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');
...
945         require(unlock_time > block.timestamp, 'Can only lock until time in the future');
946         require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 4 years max');
...
982         require(_locked.amount > 0, 'No existing lock found');
983         require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');
...
996         require(_locked.end > block.timestamp, 'Lock expired');
997         require(_locked.amount > 0, 'Nothing is locked');
998         require(unlock_time > _locked.end, 'Can only increase lock duration');
999         require(unlock_time <= block.timestamp + MAXTIME, 'Voting lock can be 4 years max');
...
1008         require(attachments[_tokenId] == 0 && !voted[_tokenId], 'attached');
...
1011         require(block.timestamp >= _locked.end, "The lock didn't expire");
...
1082         require(idToOwner[_tokenId] != address(0), 'Query for nonexistent token');
...
1227         require(_isApprovedOrOwner(msg.sender, _tokenId), 'caller is not owner nor approved');

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:538

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:894

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:928-929

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:945-946

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:982-983

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:996-999

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1008

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1011

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1082

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1227

  1. a. ++i costs less gas than i++, especially when it’s used in for-loops (--i/i-- too) Gas saved: 7080 + gas saved when calling the method, depending on iterations of the loop.
    • 2022-07-golom/contracts/rewards/GolomTrader.sol:415,
415        for (uint256 i = 0; i < proof.length; i++) {

2022-07-golom/contracts/rewards/GolomTrader.sol:415

- 2022-07-golom/contracts/rewards/RewardDistributor.sol:143,157,180,183,226,258,273
143        for (uint256 index = 0; index < epochs.length; index++) {
...
157        for (uint256 index = 0; index < epochs.length; index++) {
...
180        for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {
...
183            for (uint256 index = 0; index < epochs.length; index++) {
...
226        for (uint256 index = 0; index < epoch; index++) {
...
258        for (uint256 index = 0; index < epoch; index++) {
...
273        for (uint256 index = 0; index < epoch; index++) {

2022-07-golom/contracts/rewards/RewardDistributor.sol:143

2022-07-golom/contracts/rewards/RewardDistributor.sol:157

2022-07-golom/contracts/rewards/RewardDistributor.sol:180

2022-07-golom/contracts/rewards/RewardDistributor.sol:183

2022-07-golom/contracts/rewards/RewardDistributor.sol:226

2022-07-golom/contracts/rewards/RewardDistributor.sol:258

2022-07-golom/contracts/rewards/RewardDistributor.sol:273

- 2022-07-golom/contracts/vote-escrow/TokenUriHelper.sol:138
138            digits++;

2022-07-golom/contracts/vote-escrow/TokenUriHelper.sol:138

- 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:171,189,199
171        for (uint256 index = 0; index < delegated.length; index++) {
...
189        for (uint256 index = 0; index < delegatednft.length; index++) {
...
199        for (uint256 i; i < _array.length; i++) {

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:171

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:189

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:199

b. i = i + 1 even more gas efficient than ++i Gas saved: 23739 + gas saved when calling the method, depending on iterations of the loop.

  • 2022-07-golom/contracts/rewards/GolomTrader.sol:324
324        uint256 newNonce = ++nonces[msg.sender];

2022-07-golom/contracts/rewards/GolomTrader.sol:324

  • 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:745,948,1044,1115,1167
745            for (uint256 i = 0; i < 255; ++i) {
...
948        ++tokenId;
...
1044        for (uint256 i = 0; i < 128; ++i) {
...
1115        for (uint256 i = 0; i < 128; ++i) {
...
1167        for (uint256 i = 0; i < 255; ++i) {

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:745

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:948

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1044

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1115

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1167

c. Use unchecked{} in the for/while loops Gas Saved: 55668 + gas saved when calling the method, depending on iterations of the loop.

all these cases can be rewritten (if overflow is not possible) with::

unchecked{
    <a> = <a> + 1;
}
  1. Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate Gas saved: 59326
    • 2022-07-golom/contracts/rewards/RewardDistributor.sol:60-67
60        mapping(uint256 => uint256) public epochTotalFee; // total fee of epoch
61        mapping(uint256 => uint256) public rewardTrader; // reward minted each epoc for trader
62        mapping(uint256 => uint256) public rewardExchange; // reward minted each epoc for exhange
63        mapping(uint256 => uint256) public rewardLP; // reward minted each epoc for LP
64        mapping(uint256 => uint256) public rewardStaker; // reward minted each epoc for stakers
65        mapping(uint256 => uint256) public epochBeginTime; // what time previous epoch ended
66        mapping(uint256 => uint256) public claimedUpto; // epoch upto which tokenid has claimed
67        mapping(uint256 => mapping(uint256 => uint256)) public claimed; // epoch upto which tokenid has claimed

2022-07-golom/contracts/rewards/RewardDistributor.sol:60-67

fix

struct EpochInfo {
        uint256 totalFee;
        uint256 rewardTrader;
        uint256 rewardExchange;
        uint256 rewardLp;
        uint256 rewardStaker;
        uint256 beginTime;
        uint256 claimedUpTo;
        mapping(uint256 => uint256) claimed;
    }
mapping(uint256 => EpochInfo) public epochesInfo;
...
**a lot of editing accesses and assignments (I have this file in edited form if you need)**
  1. dead code + never used + misc. Gas saved: 23229 Replace secsInDay with built-in keyword days (240 gas)
    • 2022-07-golom/contracts/rewards/RewardDistributor.sol:57,106
57      uint256 constant secsInDay = 24 * 60 * 60;
...
106     if (block.timestamp > startTime + (epoch) * secsInDay) { //@audit change with days

2022-07-golom/contracts/rewards/RewardDistributor.sol:57

2022-07-golom/contracts/rewards/RewardDistributor.sol:106

Code(178-180) unreachable due to previous check(177) (12722 gas)

  • 2022-07-golom/contracts/core/GolomTrader.sol:177-180
177   require(signaturesigner == o.signer, 'invalid signature');
178   if (signaturesigner != o.signer) {// @audit unreachible
179         return (0, hashStruct, 0);
180   }

2022-07-golom/contracts/core/GolomTrader.sol:177-180

Unused variables can be removed (10267 gas)

  • 2022-07-golom/contracts/core/GolomTrader.sol:72
72    address public governance;

2022-07-golom/contracts/core/GolomTrader.sol:72

  1. Require()/revert() strings longer than 32 bytes cost extra gas (12 instances) Gas saved: 15259

    • 2022-07-golom/contracts/governance/GolomToken.sol:24
24           require(msg.sender == minter, 'GolomToken: only reward distributor can enable');

2022-07-golom/contracts/governance/GolomToken.sol:24

  • 2022-07-golom/contracts/rewards/RewardDistributor.sol:181,292,309
181          require(tokenowner == ve.ownerOf(tokenids[tindex]), 'Can only claim for a single Address together');
...
292          require(traderEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');
...
309          require(voteEscrowEnableDate <= block.timestamp, 'RewardDistributor: time not over yet');

2022-07-golom/contracts/rewards/RewardDistributor.sol:181

2022-07-golom/contracts/rewards/RewardDistributor.sol:292

2022-07-golom/contracts/rewards/RewardDistributor.sol:309

  • 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:608,929,945,983,1227
608          revert('ERC721: transfer to non ERC721Receiver implementer');
...
929          require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');
...
945          require(unlock_time > block.timestamp, 'Can only lock until time in the future');
...
983          require(_locked.end > block.timestamp, 'Cannot add to expired lock. Withdraw');
...
1227          require(_isApprovedOrOwner(msg.sender, _tokenId), 'caller is not owner nor approved');

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:608

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:929

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:945

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:983

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1227

  • 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:73,130,186
73          require(this.balanceOfNFT(tokenId) >= MIN_VOTING_POWER_REQUIRED, 'VEDelegation: Need more voting power');
...
130          require(blockNumber < block.number, 'VEDelegation: not yet determined');
...
186         require(blockNumber < block.number, 'VEDelegation: not yet determined');

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:73

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:130

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:186

  1. Using bools for storage incurs overhead (11 instances) Gas saved: 12624
  • 2022-07-golom/contracts/rewards/GolomTrader.sol:55
55        bool isERC721; // standard of the collection , if 721 then true , if 1155 then false

2022-07-golom/contracts/rewards/GolomTrader.sol:55

  • 2022-07-golom/contracts/governance/GolomToken.sol:20,21
20        bool public isAirdropMinted;
21        bool public isGenesisRewardMinted;

2022-07-golom/contracts/governance/GolomToken.sol:20-21

  • 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:314,341,344,439-441,650-651
314        mapping(uint256 => bool) public voted;

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:314

  1. It costs more gas to initialize non-constant/non-immutable variables to zero than to let the default of zero be applied (34 instances) Gas saved: 4212 deployment gas + gas saved when calling the method, depending on iterations of the loop.
    • 2022-07-golom/contracts/rewards/RewardDistributor.sol:45,142,143,156,157,175,176,180,183,222,223,226,257,258,272,273
45      uint256 public epoch = 0;
...
142        uint256 reward = 0;
143                 for (uint256 index = 0; index < epochs.length; index++) {
...
156                 uint256 reward = 0;
157        for (uint256 index = 0; index < epochs.length; index++) {
...
175        uint256 reward = 0;
176        uint256 rewardEth = 0;
...
180        for (uint256 tindex = 0; tindex < tokenids.length; tindex++) {
...
183            for (uint256 index = 0; index < epochs.length; index++) {
...
222        uint256 reward = 0;
223        uint256 rewardEth = 0;
...
226        for (uint256 index = 0; index < epoch; index++) {
...
257        uint256 reward = 0;
258        for (uint256 index = 0; index < epoch; index++) {
...
272        uint256 reward = 0;
273        for (uint256 index = 0; index < epoch; index++) {

2022-07-golom/contracts/rewards/RewardDistributor.sol:45

2022-07-golom/contracts/rewards/RewardDistributor.sol:142

2022-07-golom/contracts/rewards/RewardDistributor.sol:156

2022-07-golom/contracts/rewards/RewardDistributor.sol:175

2022-07-golom/contracts/rewards/RewardDistributor.sol:180

2022-07-golom/contracts/rewards/RewardDistributor.sol:183

2022-07-golom/contracts/rewards/RewardDistributor.sol:222

2022-07-golom/contracts/rewards/RewardDistributor.sol:226

2022-07-golom/contracts/rewards/RewardDistributor.sol:257

2022-07-golom/contracts/rewards/RewardDistributor.sol:272

  • 2022-07-golom/contracts/rewards/GolomTrader.sol:415
415        for (uint256 i = 0; i < proof.length; i++) {

2022-07-golom/contracts/rewards/GolomTrader.sol:415

  • 2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:735,745,1042,1044,1113,1115,1133,1134,1167,1211
735        uint256 block_slope = 0; // dblock/dt
...
745            for (uint256 i = 0; i < 255; ++i) {
...
1042        uint256 _min = 0;
...
1044        for (uint256 i = 0; i < 128; ++i) {
...
1113        uint256 _min = 0;
...
1115        for (uint256 i = 0; i < 128; ++i) {
...
1133        uint256 d_block = 0;
1134        uint256 d_t = 0;
...
1167        for (uint256 i = 0; i < 255; ++i) {
...
1211        uint256 dt = 0;

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:735

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:745

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1042

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1044

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1113

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1115

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1133

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1167

2022-07-golom/contracts/vote-escrow/VoteEscrowCore.sol:1211

  • 2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:50,147,170,171,188,189
50    uint256 public MIN_VOTING_POWER_REQUIRED = 0;
...
147        uint256 lower = 0;
...
170        uint256 votes = 0;
171        for (uint256 index = 0; index < delegated.length; index++) {
...
188        uint256 votes = 0;
189        for (uint256 index = 0; index < delegatednft.length; index++) {

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:50

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:147

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:170

2022-07-golom/contracts/vote-escrow/VoteEscrowDelegation.sol:188

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter