Platform: Code4rena
Start Date: 23/05/2022
Pot Size: $50,000 USDC
Total HM: 44
Participants: 99
Period: 5 days
Judge: hickuphh3
Total Solo HM: 11
Id: 129
League: ETH
Rank: 5/99
Findings: 13
Award: $2,507.33
🌟 Selected for report: 3
🚀 Solo Findings: 0
390.3586 USDC - $390.36
Users will lose funds associated with orders they've placed, to other users that cancel the order
The RubiconRouter does no validation itself of whether a user should be able to cancel an order, and instead relies on RubiconMarket to do the checks:
File: contracts/RubiconRouter.sol #1 440 function cancelForETH(uint256 id) external returns (bool outcome) { 441 (uint256 pay_amt, ERC20 pay_gem, , ) = RubiconMarket( 442 RubiconMarketAddress 443 ).getOffer(id); 444 require( 445 address(pay_gem) == wethAddress, 446 "trying to cancel a non WETH order" 447 ); 448 // Cancel order and receive WETH here in amount of pay_amt 449 outcome = RubiconMarket(RubiconMarketAddress).cancel(id); 450 WETH9(wethAddress).withdraw(pay_amt); 451 msg.sender.transfer(pay_amt); 452 }
The RubiconMarket only verifies that the message sender is the owner:
File: contracts/RubiconMarket.sol #2 215 modifier can_cancel(uint256 id) virtual { 216 require(isActive(id)); 217 require(getOwner(id) == msg.sender); 218 _; 219 }
which will always be the RubiconRouter for orders placed through the RubiconRouter:
File: contracts/RubiconMarket.sol #3 391 /// @notice Key function to make a new offer. Takes funds from the caller into market escrow. 392 function offer( 393 uint256 pay_amt, 394 ERC20 pay_gem, 395 uint256 buy_amt, 396 ERC20 buy_gem 397 ) public virtual can_offer synchronized returns (uint256 id) { 398 require(uint128(pay_amt) == pay_amt); 399 require(uint128(buy_amt) == buy_amt); 400 require(pay_amt > 0); 401 require(pay_gem != ERC20(0x0)); 402 require(buy_amt > 0); 403 require(buy_gem != ERC20(0x0)); 404 require(pay_gem != buy_gem); 405 406 OfferInfo memory info; 407 info.pay_amt = pay_amt; 408 info.pay_gem = pay_gem; 409 info.buy_amt = buy_amt; 410 info.buy_gem = buy_gem; 411 info.owner = msg.sender;
Code inspection
Maintain a mapping from submitters to order IDs, inside RubiconRouter, and validate during cancel
#0 - bghughes
2022-06-04T00:44:17Z
Not yet used in production, good issue!
#1 - HickupHH3
2022-06-18T07:14:01Z
duplicate of #17
🌟 Selected for report: kenzo
Also found by: IllIllI, PP1004, blackscale, hansfriese
390.3586 USDC - $390.36
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L282 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L171-L177
Extra funds are stolen from the user, and become locked in the RubiconRouter for others to steal
Most of the other functions of the RubiconRouter handle fees by taking the amount being provided, using a percentage as the fee, then adding that to the total. swapEntireBalance() (and also getExpectedSwapFill()) use the user's total balance and subtract the fee:
File: contracts/RubiconRouter.sol #1 282 maxAmount.sub(buy_amt_min.mul(expectedMarketFeeBPS).div(10000)), //account for fee
File: contracts/RubiconRouter.sol #2 171 uint256 _pay = i == 0 172 ? pay_amt 173 : ( 174 currentAmount.sub( 175 currentAmount.mul(expectedMarketFeeBPS).div(10000) 176 ) 177 );
If a user has a balance of $1000 and the fee is 1%, to consume the total funds $990.0990099 should be used as the payment, and $9.90099009 as the fee since those two add up to ~$1,000. The code however uses $10 as the fee and $990 as the fee. When amounts are large, this difference can be significant
Code inspection
Divide buy_amt_min by the sum of one plus the fee decimal percentage, in order to get the correct fee
#0 - HickupHH3
2022-06-16T14:45:02Z
duplicate of #104
🌟 Selected for report: Ruhum
Also found by: IllIllI, berndartmueller, blackscale, eccentricexit, hansfriese
Users can either be under-charged or over-charged fees if the decimals of the two tokens don't match. This can also lead to the full input token amount being taken as a fee.
swapEntireBalance() charges a fee based on buy_amt_min, which is the number of tokens the user expects to receive. All other functions charge the fee on the input token amount, not the output token amount.
File: contracts/RubiconRouter.sol #1 267 function swapEntireBalance( 268 uint256 buy_amt_min, 269 address[] calldata route, // First address is what is being payed, Last address is what is being bought 270 uint256 expectedMarketFeeBPS 271 ) external returns (uint256) { 272 //swaps msg.sender entire balance in the trade 273 uint256 maxAmount = ERC20(route[0]).balanceOf(msg.sender); 274 ERC20(route[0]).transferFrom( 275 msg.sender, 276 address(this), 277 maxAmount // Account for expected fee 278 ); 279 return 280 _swap( 281 maxAmount, 282 maxAmount.sub(buy_amt_min.mul(expectedMarketFeeBPS).div(10000)), //account for fee
Consider the case where a user wants to use USDC (6 decimals) to buy 1 WETH (18 decimals) with a .02% fee
~6,000 USDC = 1 WETH 6000000000(balanceOf(msg.sender)) = 1000000000000000000(buy_amt_min) 1000000000000000000 * 0.0002 = 200000000000000 Fee: 200,000,000 USDC
It's possible to fiddle with the numbers to make the fee eat up exactly the user's balance whenever the output token has more decimals than the input token
Code inspection
Charge the fee based on maxAmount instead of buy_amt_min
#0 - bghughes
2022-06-04T21:36:01Z
Duplicate of #248
#1 - HickupHH3
2022-06-16T14:48:26Z
it's a closer duplicate of #52.
8.2687 USDC - $8.27
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L451 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L491
The use of payable.transfer() is heavily frowned upon because it can lead to the locking of funds. The transfer() call requires that the recipient has a payable callback, only provides 2300 gas for its operation. This means the following cases can cause the transfer to fail:
payable callbackpayable callback spends more than 2300 gas (which is only enough to emit something)If a user falls into one of the above categories, they'll be unable to receive funds from the pool, or cancel Ether orders, and the deposited funds will be locked forever
File: contracts/RubiconRouter.sol #1 451 msg.sender.transfer(pay_amt);
File: contracts/RubiconRouter.sol #2 491 msg.sender.transfer(withdrawnWETH);
Note that there are other instances of payable.transfer(), but these are the only ones that lock user funds
Code inspection
Use address.call{value:x}() instead
#0 - bghughes
2022-06-07T22:30:22Z
Duplicate of #82
🌟 Selected for report: berndartmueller
Also found by: 0x1f8b, 0xDjango, 0xsomeone, ACai, Bahurum, BouSalman, CertoraInc, Deivitto, Dravee, GimelSec, IllIllI, JMukesh, Kaiziron, PP1004, Ruhum, SmartSek, VAD37, WatchPug, _Adam, aez121, antonttc, blockdev, broccolirob, camden, cccz, cryptphi, defsec, dipp, ellahi, fatherOfBlocks, gzeon, horsefacts, ilan, jayjonah8, joestakey, kenta, kenzo, minhquanym, oyc_109, pauliax, pedroais, peritoflores, sashik_eth, shenwilly, simon135, throttle, xiaoming90, z3s
0.1022 USDC - $0.10
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L194-L206 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L267-L278 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L361-L366 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L412-L419
Users that have provided too many tokens in prior transactions have their excess funds (now latent) stolen, so they can't be returned by an admin
There are already latent funds in the contract, so latent funds are possible: https://optimistic.etherscan.io/address/0x7Af14ADc8Aea70f063c7eA3B2C1AD0D7A59C4bFf
Many of the functions in RubiconRouter don't check the return value of transferFrom(). There are tokens that do not revert on failure, and instead return an error status. Without having a require() to check the error status, the code allows to continue on to the RubiconMarket, where the funds are withdrawn from the router, and there the status is properly checked:
File: contracts/RubiconRouter.sol #1 194 function swap( 195 uint256 pay_amt, 196 uint256 buy_amt_min, 197 address[] calldata route, // First address is what is being payed, Last address is what is being bought 198 uint256 expectedMarketFeeBPS //20 199 ) public returns (uint256) { 200 //**User must approve this contract first** 201 //transfer needed amount here first 202 ERC20(route[0]).transferFrom( 203 msg.sender, 204 address(this), 205 pay_amt.add(pay_amt.mul(expectedMarketFeeBPS).div(10000)) // Account for expected fee 206 );
By constructing a multi-step transaction, a user can claim latent funds in a transaction of their own by doing the following:
RubiconRouterRubiconRouterRubiconRouter contract, so it can't pull more fundsswap()There are a bunch of other functions that have the same issue:
File: contracts/RubiconRouter.sol #2 267 function swapEntireBalance( 268 uint256 buy_amt_min, 269 address[] calldata route, // First address is what is being payed, Last address is what is being bought 270 uint256 expectedMarketFeeBPS 271 ) external returns (uint256) { 272 //swaps msg.sender entire balance in the trade 273 uint256 maxAmount = ERC20(route[0]).balanceOf(msg.sender); 274 ERC20(route[0]).transferFrom( 275 msg.sender, 276 address(this), 277 maxAmount // Account for expected fee 278 );
File: contracts/RubiconRouter.sol #3 361 function buyAllAmountForETH( 362 uint256 buy_amt, 363 ERC20 pay_gem, 364 uint256 max_fill_amount 365 ) external returns (uint256 fill) { 366 ERC20(pay_gem).transferFrom(msg.sender, address(this), max_fill_amount); //transfer pay here
File: contracts/RubiconRouter.sol #4 361 function buyAllAmountForETH( 362 uint256 buy_amt, 363 ERC20 pay_gem, 364 uint256 max_fill_amount 365 ) external returns (uint256 fill) { 366 ERC20(pay_gem).transferFrom(msg.sender, address(this), max_fill_amount); //transfer pay here
File: contracts/RubiconRouter.sol #5 412 function offerForETH( 413 uint256 pay_amt, //maker (ask) sell how much 414 ERC20 pay_gem, //maker (ask) sell which token 415 uint256 buy_amt, //maker (ask) buy how much 416 // ERC20 nativeETH, //maker (ask) buy which token 417 uint256 pos //position to insert offer, 0 should be used if unknown 418 ) external returns (uint256) { 419 ERC20(pay_gem).transferFrom(msg.sender, address(this), pay_amt);
See this similar issue that was found to be of Medium risk
Code inspection
require() that the transferFrom() functions return success
#0 - bghughes
2022-06-04T21:06:20Z
Duplicate of #316
🌟 Selected for report: cccz
Also found by: AlleyCat, GimelSec, IllIllI, Ruhum, berndartmueller, csanuragjain, dipp, fatherOfBlocks, gzeon, horsefacts, pedroais, shenwilly
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L454-L462 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L494-L505 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L519-L549
Users send more funds than are needed, and the excess gets locked in the contract
This is not hypothetical - there are already funds from six separate tokens stuck in the contract: https://optimistic.etherscan.io/address/0x7Af14ADc8Aea70f063c7eA3B2C1AD0D7A59C4bFf
Some functions unnecessarily allow sending more than the requested amount:
File: contracts/RubiconRouter.sol #1 454 // Deposit native ETH -> WETH pool 455 function depositWithETH(uint256 amount, address targetPool) 456 external 457 payable 458 returns (uint256 newShares) 459 { 460 IERC20 target = IBathToken(targetPool).underlyingToken(); 461 require(target == ERC20(wethAddress), "target pool not weth pool"); 462 require(msg.value >= amount, "didnt send enough eth");
File: contracts/RubiconRouter.sol #2 494 function swapWithETH( 495 uint256 pay_amt, 496 uint256 buy_amt_min, 497 address[] calldata route, // First address is what is being payed, Last address is what is being bought 498 uint256 expectedMarketFeeBPS 499 ) external payable returns (uint256) { 500 require(route[0] == wethAddress, "Initial value in path not WETH"); 501 uint256 amtWithFee = pay_amt.add( 502 pay_amt.mul(expectedMarketFeeBPS).div(10000) 503 ); 504 require( 505 msg.value >= amtWithFee,
And the functions that charge fees don't verify that the fee value is the actual fee value in use, and thus may send extra funds. Here's one example of quite a few:
File: contracts/RubiconRouter.sol #3 519 function swapForETH( 520 uint256 pay_amt, 521 uint256 buy_amt_min, 522 address[] calldata route, // First address is what is being payed, Last address is what is being bought 523 uint256 expectedMarketFeeBPS 524 ) external payable returns (uint256 fill) { 525 require( 526 route[route.length - 1] == wethAddress, 527 "target of swap is not WETH" 528 ); 529 //Transfer tokens here first and account for fee 530 require( 531 ERC20(route[0]).transferFrom( 532 msg.sender, 533 address(this), 534 pay_amt.add(pay_amt.mul(expectedMarketFeeBPS).div(10000)) 535 ), 536 "initial ERC20 transfer failed" 537 ); 538 fill = _swap( 539 pay_amt, 540 buy_amt_min, 541 route, 542 expectedMarketFeeBPS, 543 address(this) 544 ); 545 546 WETH9(wethAddress).withdraw(fill); 547 // msg.sender.transfer(fill); 548 msg.sender.transfer(fill); 549 }
Swaps may also not consume all of the input tokens, so those remainders should be refunded
See this similar prior issue that was found to be of Medium risk
Code inspection
Reject transactions where msg.value doesn't equal the amount, and look up the actual fee and use that if it's less than expectedMarketFeeBPS. Also, refund any unused funds for transactions that don't consume the full input tokens.
#0 - KenzoAgada
2022-06-04T15:32:16Z
Duplicate of #15. The warden also briefly mentions the separate issue that swaps with ERC20 tokens might not consume all funds so remainder needs to be refunded - judge will need to decide whether that should be separated to a separate issue.
#1 - bghughes
2022-06-04T22:34:31Z
Duplicate of #15
🌟 Selected for report: xiaoming90
Also found by: GimelSec, IllIllI, MaratCerby, PP1004, WatchPug, berndartmueller, blockdev, ilan
42.6857 USDC - $42.69
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L251 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L303 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L320 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L348 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L353-L356 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L357
Users will lose funds if fee-on-transfer tokens are used in a withdrawal step, after having been deposited when there was no fee
The Rubicon code does not verify that the balances available match the stored amounts, and uses intermediate contracts in hops during transfers. Each hop may incur a fee, which will change the available balance. For both the RubiconRouter and the BathToken, funds are transferred from the RubiconMarket to those contracts before being withdrawn. If there is a fee, the amount stored will not match the amount available, and the calls will revert, locking the user's funds.
File: contracts/RubiconRouter.sol #1 251 ERC20(route[route.length - 1]).transfer(to, currentAmount);
File: contracts/RubiconRouter.sol #2 303 ERC20(buy_gem).transfer(msg.sender, fill);
File: contracts/RubiconRouter.sol #3 320 ERC20(buy_gem).transfer(msg.sender, fill);
File: contracts/RubiconRouter.sol #4 348 ERC20(buy_gem).transfer(msg.sender, buy_amt);
File: contracts/rubiconPools/BathToken.sol #5 353 IERC20(filledAssetToRebalance).transfer( 354 destination, 355 rebalAmt.sub(stratReward) 356 );
File: contracts/rubiconPools/BathToken.sol #6 357 IERC20(filledAssetToRebalance).transfer(msg.sender, stratReward);
Code inspection
Measure the balance before and after transfers, and use the difference as the amount, rather than the stated amount
#0 - bghughes
2022-06-03T23:02:14Z
Global risk to any ERC-20 by this logic
#1 - HickupHH3
2022-06-23T15:45:38Z
grouping it with the FoT / deflationary issue: #112
🌟 Selected for report: kenzo
Also found by: 0x1f8b, 0xsomeone, Dravee, IllIllI, MaratCerby, berndartmueller, cryptphi, xiaoming90
42.6857 USDC - $42.69
Judge has assessed an item in Issue #203 as Medium risk. The relevant finding follows:
#0 - HickupHH3
2022-06-27T13:51:49Z
🌟 Selected for report: 0x1f8b
Also found by: CertoraInc, IllIllI, rotcivegaf, unforgiven
87.8307 USDC - $87.83
Judge has assessed an item in Issue #203 as Medium risk. The relevant finding follows:
#0 - HickupHH3
2022-06-27T13:47:17Z
dup of #38
🌟 Selected for report: IllIllI
Also found by: 0xDjango, Dravee, SmartSek, StErMi, oyc_109, pauliax, rotcivegaf, sashik_eth, shenwilly, xiaoming90
23.3384 USDC - $23.34
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathHouse.sol#L216-L229 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathHouse.sol#L334-L337 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/rubiconPools/BathToken.sol#L269-L272
There are multiple functions that the admin can call to brick various parts of the system, leading to user's funds being locked
Even if the owner is benevolent the fact that there is a rug vector available may negatively impact the protocol's reputation. See this example where a similar finding has been flagged as a high-severity issue. I've downgraded these instances to be a medium since it requires cooperation of the admin.
Here are some examples: Overwrite state:
File: contracts/rubiconPools/BathHouse.sol #1 216 /// @notice A migration function that allows the admin to write arbitrarily to tokenToBathToken 217 function adminWriteBathToken(ERC20 overwriteERC20, address newBathToken) 218 external 219 onlyAdmin 220 { 221 tokenToBathToken[address(overwriteERC20)] = newBathToken; 222 emit LogNewBathToken( 223 address(overwriteERC20), 224 newBathToken, 225 address(0), 226 block.timestamp, 227 msg.sender 228 ); 229 }
Steal new funds with new malicious market
File: contracts/rubiconPools/BathHouse.sol #2 334 /// @notice Admin-only function to set a Bath Token's target Rubicon Market 335 function setMarket(address newMarket) external onlyAdmin { 336 RubiconMarketAddress = newMarket; 337 }
Add a ton of bonus tokens, making withdrawals break because of unbounded for-loop
File: contracts/rubiconPools/BathToken.sol #3 269 /// @notice Admin-only function to add a bonus token to bonusTokens for pool incentives 270 function setBonusToken(address newBonusERC20) external onlyBathHouse { 271 bonusTokens.push(newBonusERC20); 272 }
Code inspection
Validate input arguments and require specific upgrade paths for each contract, not just allowing the admin to set whatever they decide, add limit to number of bonus tokens
#0 - bghughes
2022-06-03T20:49:57Z
Centralization risk is acknowledged. Duplicate of #344
https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L290-L297 https://github.com/code-423n4/2022-05-rubicon/blob/8c312a63a91193c6a192a9aab44ff980fbfd7741/contracts/RubiconRouter.sol#L307-L316
Availability of the protocol is impacted. Also, customers waste gas trying to call functions that don't work
The two functions below call balanceOf() on the msg.sender, but the RubiconMarket pulls funds from the caller, which is the RubiconRouter, not msg.sender, so the amount will never be available
File: contracts/RubiconRouter.sol #1 290 function maxBuyAllAmount( 291 ERC20 buy_gem, 292 ERC20 pay_gem, 293 uint256 max_fill_amount 294 ) external returns (uint256 fill) { 295 //swaps msg.sender's entire balance in the trade 296 uint256 maxAmount = ERC20(buy_gem).balanceOf(msg.sender); 297 fill = RubiconMarket(RubiconMarketAddress).buyAllAmount(
File: contracts/RubiconRouter.sol #2 307 function maxSellAllAmount( 308 ERC20 pay_gem, 309 ERC20 buy_gem, 310 uint256 min_fill_amount 311 ) external returns (uint256 fill) { 312 //swaps msg.sender entire balance in the trade 313 uint256 maxAmount = ERC20(buy_gem).balanceOf(msg.sender); 314 fill = RubiconMarket(RubiconMarketAddress).sellAllAmount( 315 pay_gem, 316 maxAmount,
Code inspection
Transfer funds from the user before calling buyAllAmount() or sellAllAmount()
#0 - bghughes
2022-06-04T21:31:01Z
Duplicate of #282
🌟 Selected for report: IllIllI
Also found by: 0x1337, 0x1f8b, 0x4non, 0xDjango, 0xKitsune, 0xNazgul, 0xf15ers, ACai, AlleyCat, Bahurum, BouSalman, CertoraInc, Chom, Dravee, ElKu, FSchmoede, Funen, GimelSec, Hawkeye, JC, JMukesh, Kaiziron, MaratCerby, Metatron, PP1004, Picodes, Ruhum, SmartSek, StErMi, TerrierLover, UVvirus, UnusualTurtle, WatchPug, Waze, _Adam, asutorufos, berndartmueller, blackscale, blockdev, broccolirob, c3phas, catchup, cryptphi, csanuragjain, defsec, delfin454000, dipp, eccentricexit, ellahi, fatherOfBlocks, gzeon, hansfriese, horsefacts, hubble, ilan, joestakey, kebabsec, minhquanym, oyc_109, parashar, pauliax, rotcivegaf, sach1r0, sashik_eth, shenwilly, simon135, sorrynotsorry, sseefried, throttle, unforgiven, xiaoming90
509.339 USDC - $509.34
| Issue | Instances | |
|---|---|---|
| 1 | Unsafe use of transfer()/transferFrom() with IERC20 | 7 |
| 2 | Return values of transfer()/transferFrom() not checked | 7 |
| 3 | Unused/empty receive()/fallback() function | 3 |
| 4 | Incorrect implementation of DOMAIN_SEPARATOR | 1 |
| 5 | Return unused fees | 5 |
| 6 | Migrations should do some validation | 1 |
| 7 | A second call to the function may revert | 1 |
| 8 | Front-runable initializer | 1 |
| 9 | Vulnerable to cross-chain replay attacks due to static DOMAIN_SEPARATOR | 1 |
| 10 | Contracts should extend interfaces they extend | 1 |
| 11 | NatSpec incorrect | 1 |
| 12 | Misleading function name | 1 |
| 13 | Missing checks for address(0x0) when assigning values to address state variables | 16 |
Total: 46 instances over 13 issues
| Issue | Instances | |
|---|---|---|
| 1 | Consider addings checks for signature malleability | 1 |
| 2 | No use of two-phase ownership transfers | 1 |
| 3 | Return values of approve() not checked | 3 |
| 4 | Adding a return statement when the function defines a named return variable, is redundant | 2 |
| 5 | require()/revert() statements should have descriptive reason strings | 55 |
| 6 | public functions not called by the contract should be declared external instead | 26 |
| 7 | Non-assembly method available | 1 |
| 8 | 2**<n> - 1 should be re-written as type(uint<n>).max | 5 |
| 9 | type(uint<n>).max should be used instead of uint<n>(-1) | 1 |
| 10 | constants should be defined rather than using magic numbers | 43 |
| 11 | Redundant cast | 1 |
| 12 | Missing event for critical parameter change | 13 |
| 13 | Use a more recent version of solidity | 2 |
| 14 | Use a more recent version of solidity | 3 |
| 15 | Use scientific notation (e.g. 1e18) rather than exponentiation (e.g. 10**18) | 10 |
| 16 | Inconsistent spacing in comments | 127 |
| 17 | Variable names that consist of all capital letters should be reserved for const/immutable variables | 1 |
| 18 | Typos | 17 |
| 19 | NatSpec is incomplete | 1 |
| 20 | Event is missing indexed fields | 33 |
| 21 | Not using the named return variables anywhere in the function is confusing | 15 |
Total: 361 instances over 21 issues
transfer()/transferFrom() with IERC20Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether (USDT)'s transfer() and transferFrom() functions do not return booleans as the specification requires, and instead have no return value. When these sorts of tokens are cast to IERC20, their function signatures do not match and therefore the calls made, revert. Use OpenZeppelin’s SafeERC20's safeTransfer()/safeTransferFrom() instead
There are 7 instances of this issue:
File: contracts/rubiconPools/BathPair.sol 601: IERC20(asset).transfer(msg.sender, booty); 615: IERC20(quote).transfer(msg.sender, booty);
File: contracts/rubiconPools/BathToken.sol 353 IERC20(filledAssetToRebalance).transfer( 354 destination, 355 rebalAmt.sub(stratReward) 356: ); 357: IERC20(filledAssetToRebalance).transfer(msg.sender, stratReward); 565: underlyingToken.transferFrom(msg.sender, address(this), assets); 602: underlyingToken.transfer(feeTo, _fee); 605: underlyingToken.transfer(receiver, amountWithdrawn);
transfer()/transferFrom() not checkedNot all IERC20 implementations revert() when there's a failure in transfer()/transferFrom(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually making a payment
There are 7 instances of this issue:
File: contracts/rubiconPools/BathPair.sol 601: IERC20(asset).transfer(msg.sender, booty); 615: IERC20(quote).transfer(msg.sender, booty);
File: contracts/rubiconPools/BathToken.sol 353 IERC20(filledAssetToRebalance).transfer( 354 destination, 355 rebalAmt.sub(stratReward) 356: ); 357: IERC20(filledAssetToRebalance).transfer(msg.sender, stratReward); 565: underlyingToken.transferFrom(msg.sender, address(this), assets); 602: underlyingToken.transfer(feeTo, _fee); 605: underlyingToken.transfer(receiver, amountWithdrawn);
receive()/fallback() functionIf the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth)))
There are 3 instances of this issue:
File: contracts/peripheral_contracts/BathBuddy.sol #1 69: receive() external payable {}
File: contracts/RubiconRouter.sol #2 37: receive() external payable {}
File: contracts/RubiconRouter.sol #3 39: fallback() external payable {}
DOMAIN_SEPARATORI confirmed with the sponsor that the v1 being appended after the separtor was constructed was an attempt to help with future upgrades. However, the purpose of the DOMAIN_SEPARATOR is to actually ensure that signatures pre-upgrade are invalid post-upgrade. Users shouldn't be able to double spend - once on the old contract, and once on the new one. It looks like this behavior was done in the previous version of Rubicon as well. While no funds are at risk because the separator is used with a nonce that is currently kept at its same storage slot, there's nothing guaranteeing this for future upgrades, so it's best to fix the separator to use the actual token name.
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 199 DOMAIN_SEPARATOR = keccak256( 200 abi.encode( 201 keccak256( 202 "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" 203 ), 204 keccak256(bytes(name)), 205 keccak256(bytes("1")), 206 chainId, 207 address(this) 208 ) 209 ); 210: name = string(abi.encodePacked(_symbol, (" v1")));
The expectedMarketFeeBPS argument to a bunch of functions is a user projection. The actual fee is a static value set by the admin, not a dynamic one, so the difference between the two values should be returned, as is done for some of the other token transfers in this contract.
There are 5 instances of this issue:
File: contracts/RubiconRouter.sol 194 function swap( 195 uint256 pay_amt, 196 uint256 buy_amt_min, 197 address[] calldata route, // First address is what is being payed, Last address is what is being bought 198 uint256 expectedMarketFeeBPS //20 199: ) public returns (uint256) { 267 function swapEntireBalance( 268 uint256 buy_amt_min, 269 address[] calldata route, // First address is what is being payed, Last address is what is being bought 270 uint256 expectedMarketFeeBPS 271: ) external returns (uint256) { 325 function buyAllAmountWithETH( 326 ERC20 buy_gem, 327 uint256 buy_amt, 328 uint256 max_fill_amount, 329 uint256 expectedMarketFeeBPS 330: ) external payable returns (uint256 fill) { 494 function swapWithETH( 495 uint256 pay_amt, 496 uint256 buy_amt_min, 497 address[] calldata route, // First address is what is being payed, Last address is what is being bought 498 uint256 expectedMarketFeeBPS 499: ) external payable returns (uint256) { 519 function swapForETH( 520 uint256 pay_amt, 521 uint256 buy_amt_min, 522 address[] calldata route, // First address is what is being payed, Last address is what is being bought 523 uint256 expectedMarketFeeBPS 524: ) external payable returns (uint256 fill) {
At the very least, a new bath token should have the same underlying to make sure funds can be taken out
There is 1 instance of this issue:
File: contracts/rubiconPools/BathHouse.sol #1 216 /// @notice A migration function that allows the admin to write arbitrarily to tokenToBathToken 217 function adminWriteBathToken(ERC20 overwriteERC20, address newBathToken) 218 external 219 onlyAdmin 220 { 221 tokenToBathToken[address(overwriteERC20)] = newBathToken; 222 emit LogNewBathToken( 223 address(overwriteERC20), 224 newBathToken, 225 address(0), 226 block.timestamp, 227 msg.sender 228 ); 229: }
Some ERC20 tokens, such as Tether, revert() if approve() is called a second time without having called approve(0) first
There is 1 instance of this issue:
File: contracts/RubiconRouter.sol #1 157: ERC20(toApprove).approve(RubiconMarketAddress, 2**256 - 1);
There is nothing preventing another account from calling the initializer before the contract owner. In the best case, the owner is forced to waste gas and re-deploy. In the worst case, the owner does not notice that his/her call reverts, and everyone starts using a contract under the control of an attacker
There is 1 instance of this issue:
File: contracts/RubiconRouter.sol #1 41: function startErUp(address _theTrap, address payable _weth) external {
See this issue from a prior contest for details
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 199 DOMAIN_SEPARATOR = keccak256( 200 abi.encode( 201 keccak256( 202 "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" 203 ), 204 keccak256(bytes(name)), 205 keccak256(bytes("1")), 206 chainId, 207 address(this) 208: )
Extending an interface ensures that all function signatures are correct, and catches mistakes introduced (e.g. through errant keystrokes)
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 387: /// @notice * EIP 4626 *
The NatSpec does not match what is being done by the function
There is 1 instance of this issue:
File: contracts/rubiconPools/BathHouse.sol #1 285: /// @notice Admin-only function to set a Bath Token's timeDelay
setBonusToken() is actually appending a new bonus token to the previous list of bonus tokens. The function name should reflect the action it's taking, or else reviewers and users will get confused about what the code is actually doing
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 269 /// @notice Admin-only function to add a bonus token to bonusTokens for pool incentives 270 function setBonusToken(address newBonusERC20) external onlyBathHouse { 271 bonusTokens.push(newBonusERC20); 272: }
address(0x0) when assigning values to address state variablesThere are 16 instances of this issue:
File: contracts/RubiconMarket.sol 556: feeTo = _feeTo; 1252: AqueductAddress = _Aqueduct; 1257: feeTo = newFeeTo;
File: contracts/rubiconPools/BathHouse.sol 119: RubiconMarketAddress = market; 121: newBathTokenImplementation = _newBathTokenImplementation; 122: proxyManager = _proxyAdmin; 249: approvedPairContract = newPair; 254: admin = newAdmin; 260: newBathTokenImplementation = newImplementation; 336: RubiconMarketAddress = newMarket;
File: contracts/rubiconPools/BathPair.sol 126: bathHouse = _bathHouse;
File: contracts/rubiconPools/BathToken.sol 192: RubiconMarketAddress = market; 246: RubiconMarketAddress = newRubiconMarket; 251: bathHouse = newBathHouse;
File: contracts/RubiconRouter.sol 43: RubiconMarketAddress = _theTrap; 44: wethAddress = _weth;
Use OpenZeppelin's ECDSA contract rather than calling ecrecover() directly
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 739: address recoveredAddress = ecrecover(digest, v, r, s);
Consider adding a two-phase transfer, where the current owner nominates the next owner, and the next owner has to call accept*() to become the new owner. This prevents passing the ownership to an account that is unable to use it.
There is 1 instance of this issue:
File: contracts/rubiconPools/BathHouse.sol #1 253 function setBathHouseAdmin(address newAdmin) external onlyAdmin { 254 admin = newAdmin; 255: }
approve() not checkedNot all IERC20 implementations revert() when there's a failure in approve(). The function signature has a boolean return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything
There are 3 instances of this issue:
File: contracts/rubiconPools/BathToken.sol #1 214: IERC20(address(token)).approve(RubiconMarketAddress, 2**256 - 1);
File: contracts/rubiconPools/BathToken.sol #2 256: underlyingToken.approve(RubiconMarketAddress, 2**256 - 1);
File: contracts/RubiconRouter.sol #3 465: target.approve(targetPool, amount);
return statement when the function defines a named return variable, is redundantThere are 2 instances of this issue:
File: contracts/rubiconPools/BathPair.sol #1 315: return _index;
File: contracts/RubiconRouter.sol #2 379: return fill;
require()/revert() statements should have descriptive reason stringsThere are 55 instances of this issue:
File: contracts/RubiconMarket.sol 210: require(isActive(id)); 216: require(isActive(id)); 217: require(getOwner(id) == msg.sender); 226: require(!locked); 361: require(_offer.pay_gem.transfer(_offer.owner, _offer.pay_amt)); 379: require(cancel(uint256(id))); 398: require(uint128(pay_amt) == pay_amt); 399: require(uint128(buy_amt) == buy_amt); 400: require(pay_amt > 0); 401: require(pay_gem != ERC20(0x0)); 402: require(buy_amt > 0); 403: require(buy_gem != ERC20(0x0)); 404: require(pay_gem != buy_gem); 416: require(pay_gem.transferFrom(msg.sender, address(this), pay_amt)); 432: require(buy(uint256(id), maxTakeAmount)); 453: require(!isClosed()); 459: require(isActive(id)); 460: require(!isClosed()); 466: require(isActive(id)); 467: require((msg.sender == getOwner(id)) || isClosed()); 591: require(buy(uint256(id), maxTakeAmount)); 595: require(cancel(uint256(id))); 646: require(_dust[address(pay_gem)] <= pay_amt); 687: require(_unsort(id)); 689: require(_hide(id)); 702: require(!isOfferSorted(id)); //make sure offers[id] is not yet sorted 703: require(isActive(id)); //make sure offers[id] is active 715 require( 716 !isActive(id) && 717 _rank[id].delb != 0 && 718 _rank[id].delb < block.number - 10 719: ); 840: require(offerId != 0); //Fails if there are not more offers 865: require(fill_amt >= min_fill_amount); 879: require(offerId != 0); 906: require(fill_amt <= max_fill_amount); 921: require(offerId != 0); //Fails if there are not enough offers to complete 945: require(offerId != 0); //Fails if there are not enough offers to complete 960: require(buyEnabled); 970: require(super.buy(id, amount)); 985: require(id > 0); 1002: require(id > 0); 1119: require(_dust[address(pay_gem)] <= pay_amt); 1131: require(isActive(id)); 1175: require(_span[pay_gem][buy_gem] > 0); 1177 require( 1178 _rank[id].delb == 0 && //assert id is in the sorted list 1179 isOfferSorted(id) 1180: ); 1184: require(_rank[_rank[id].next].prev == id); 1193: require(_rank[_rank[id].prev].next == id); 1209: require(!isOfferSorted(id)); //make sure offer id is not in sorted offers list
File: contracts/rubiconPools/BathHouse.sol 88: require(msg.sender == admin); 104: require(!initialized); 110: require(_reserveRatio <= 100); 111: require(_reserveRatio > 0); 280: require(rr <= 100); 281: require(rr > 0);
File: contracts/rubiconPools/BathPair.sol 118: require(!initialized); 143: require(msg.sender == bathHouse);
File: contracts/rubiconPools/BathToken.sol 186: require(!initialized);
File: contracts/RubiconRouter.sol 42: require(!started);
public functions not called by the contract should be declared external insteadContracts are allowed to override their parents' functions and change the visibility from external to public.
There are 26 instances of this issue:
File: contracts/RubiconMarket.sol 240 function getOffer(uint256 id) 241 public 242 view 243 returns ( 244 uint256, 245 ERC20, 246 uint256, 247: ERC20 475: function getTime() public view returns (uint64) { 551: function initialize(bool _live, address _feeTo) public { 697 function insert( 698 uint256 id, //maker (ask) id 699 uint256 pos //position to insert into 700: ) public returns (bool) { 798 function getOfferCount(ERC20 sell_gem, ERC20 buy_gem) 799 public 800 view 801: returns (uint256) 811: function getFirstUnsortedOffer() public view returns (uint256) { 817: function getNextUnsortedOffer(uint256 id) public view returns (uint256) {
File: contracts/rubiconPools/BathHouse.sol 359: function getBPSToStrats() public view returns (uint8) { 382: function isApprovedPair(address pair) public view returns (bool outcome) {
File: contracts/rubiconPools/BathPair.sol 631 function getOutstandingStrategistTrades( 632 address asset, 633 address quote, 634 address strategist 635: ) public view returns (uint256[] memory) {
File: contracts/rubiconPools/BathToken.sol 383: function asset() public view returns (address assetTokenAddress) { 416 function maxDeposit(address receiver) 417 public 418 pure 419: returns (uint256 maxAssets) 425 function previewDeposit(uint256 assets) 426 public 427 view 428: returns (uint256 shares) 435: function deposit(uint256 assets) public returns (uint256 shares) { 441 function deposit(uint256 assets, address receiver) 442 public 443: returns (uint256 shares) 450: function maxMint(address receiver) public pure returns (uint256 maxShares) { 464 function mint(uint256 shares, address receiver) 465 public 466: returns (uint256 assets) 475 function maxWithdraw(address owner) 476 public 477 view 478: returns (uint256 maxAssets) 505 function withdraw( 506 uint256 assets, 507 address receiver, 508 address owner 509: ) public returns (uint256 shares) { 525: function maxRedeem(address owner) public view returns (uint256 maxShares) { 531 function previewRedeem(uint256 shares) 532 public 533 view 534: returns (uint256 assets) 542 function redeem( 543 uint256 shares, 544 address receiver, 545 address owner 546: ) public returns (uint256 assets) {
File: contracts/RubiconRouter.sol 55 function getBookFromPair( 56 ERC20 asset, 57 ERC20 quote, 58 uint256 topNOrders 59 ) 60 public 61 view 62 returns ( 63 uint256[3][] memory, 64 uint256[3][] memory, 65: uint256 129 function getBestOfferAndInfo(address asset, address quote) 130 public 131 view 132 returns ( 133 uint256, //id 134 uint256, 135 ERC20, 136 uint256, 137: ERC20 161 function getExpectedSwapFill( 162 uint256 pay_amt, 163 uint256 buy_amt_min, 164 address[] calldata route, // First address is what is being payed, Last address is what is being bought 165 uint256 expectedMarketFeeBPS //20 166: ) public view returns (uint256 fill_amt) { 194 function swap( 195 uint256 pay_amt, 196 uint256 buy_amt_min, 197 address[] calldata route, // First address is what is being payed, Last address is what is being bought 198 uint256 expectedMarketFeeBPS //20 199: ) public returns (uint256) {
assembly{ id := chainid() } => uint256 id = block.chainid;
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 197: chainId := chainid()
2**<n> - 1 should be re-written as type(uint<n>).maxEarlier versions of solidity can use uint<n>(-1) instead. Expressions not including the - 1 can often be re-written to accomodate the change (e.g. by using a > rather than a >=, which will also save some gas)
There are 5 instances of this issue:
File: contracts/rubiconPools/BathToken.sol 214: IERC20(address(token)).approve(RubiconMarketAddress, 2**256 - 1); 256: underlyingToken.approve(RubiconMarketAddress, 2**256 - 1); 421: maxAssets = 2**256 - 1; // No limit on deposits in current implementation = Max UINT 451: maxShares = 2**256 - 1; // No limit on shares that could be created via deposit in current implementation - Max UINT
File: contracts/RubiconRouter.sol 157: ERC20(toApprove).approve(RubiconMarketAddress, 2**256 - 1);
type(uint<n>).max should be used instead of uint<n>(-1)There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 703: if (allowance[from][msg.sender] != uint256(-1)) {
constants should be defined rather than using magic numbersThere are 43 instances of this issue:
File: contracts/RubiconMarket.sol /// @audit 10000 296: uint256 fee = mul(spend, feeBPS) / 10000; /// @audit 0x0 401: require(pay_gem != ERC20(0x0)); /// @audit 0x0 403: require(buy_gem != ERC20(0x0)); /// @audit 20 562: feeBPS = 20; /// @audit 9 857: pay_amt * 10**9, /// @audit 9 859: ) / 10**9; /// @audit 9 898: buy_amt * 10**9, /// @audit 9 900: ) / 10**9 /// @audit 9 927: pay_amt * 10**9, /// @audit 9 929: ) / 10**9 /// @audit 9 951: buy_amt * 10**9, /// @audit 9 953: ) / 10**9
File: contracts/rubiconPools/BathHouse.sol /// @audit 100 110: require(_reserveRatio <= 100); /// @audit 20 116: bpsToStrategists = 20; /// @audit 100 280: require(rr <= 100);
File: contracts/rubiconPools/BathPair.sol /// @audit 0x0000000000000000000000000000000000000000 122: address(0x0000000000000000000000000000000000000000) && /// @audit 100 179: ).div(100) <= IERC20(underlyingAsset).balanceOf(bathAssetAddress), /// @audit 100 187: ).div(100) <= IERC20(underlyingQuote).balanceOf(bathQuoteAddress), /// @audit 10000 556: amount.sub((stratRewardBPS.mul(amount)).div(10000)),
File: contracts/rubiconPools/BathToken.sol /// @audit 256 214: IERC20(address(token)).approve(RubiconMarketAddress, 2**256 - 1); /// @audit 3 218: feeBPS = 3; //Fee set to 3 BPS initially /// @audit 256 256: underlyingToken.approve(RubiconMarketAddress, 2**256 - 1); /// @audit 10000 352: uint256 stratReward = (stratProportion.mul(rebalAmt)).div(10000); /// @audit 256 421: maxAssets = 2**256 - 1; // No limit on deposits in current implementation = Max UINT /// @audit 256 451: maxShares = 2**256 - 1; // No limit on shares that could be created via deposit in current implementation - Max UINT /// @audit 10000 498: uint256 _fee = assets.mul(feeBPS).div(10000); /// @audit 10000 537: uint256 _fee = r.mul(feeBPS).div(10000); /// @audit 10000 599: uint256 _fee = r.mul(feeBPS).div(10000);
File: contracts/peripheral_contracts/BathBuddy.sol /// @audit 10000 106: uint256 _fee = amount.mul(poolFee).div(10000);
File: contracts/RubiconRouter.sol /// @audit 3 63: uint256[3][] memory, /// @audit 3 64: uint256[3][] memory, /// @audit 3 68: uint256[3][] memory asks = new uint256[3][](topNOrders); /// @audit 3 68: uint256[3][] memory asks = new uint256[3][](topNOrders); /// @audit 3 69: uint256[3][] memory bids = new uint256[3][](topNOrders); /// @audit 3 69: uint256[3][] memory bids = new uint256[3][](topNOrders); /// @audit 256 157: ERC20(toApprove).approve(RubiconMarketAddress, 2**256 - 1); /// @audit 10000 175: currentAmount.mul(expectedMarketFeeBPS).div(10000) /// @audit 10000 205: pay_amt.add(pay_amt.mul(expectedMarketFeeBPS).div(10000)) // Account for expected fee /// @audit 10000 233: currentAmount.mul(expectedMarketFeeBPS).div(10000) /// @audit 10000 282: maxAmount.sub(buy_amt_min.mul(expectedMarketFeeBPS).div(10000)), //account for fee /// @audit 10000 334: max_fill_amount.mul(expectedMarketFeeBPS).div(10000) /// @audit 10000 502: pay_amt.mul(expectedMarketFeeBPS).div(10000) /// @audit 10000 534: pay_amt.add(pay_amt.mul(expectedMarketFeeBPS).div(10000))
The type of the variable is the same as the type to which the variable is being cast
There is 1 instance of this issue:
File: contracts/RubiconRouter.sol #1 /// @audit address(wethAddress) 331: address _weth = address(wethAddress);
There are 13 instances of this issue:
File: contracts/RubiconMarket.sol 1231 function setFeeBPS(uint256 _newFeeBPS) external auth returns (bool) { 1232 feeBPS = _newFeeBPS; 1233 return true; 1234: } 1237 function setAqueductDistributionLive(bool live) 1238 external 1239 auth 1240 returns (bool) 1241 { 1242 AqueductDistributionLive = live; 1243 return true; 1244: } 1247 function setAqueductAddress(address _Aqueduct) 1248 external 1249 auth 1250 returns (bool) 1251 { 1252 AqueductAddress = _Aqueduct; 1253 return true; 1254: } 1256 function setFeeTo(address newFeeTo) external auth returns (bool) { 1257 feeTo = newFeeTo; 1258 return true; 1259: }
File: contracts/rubiconPools/BathHouse.sol 253 function setBathHouseAdmin(address newAdmin) external onlyAdmin { 254 admin = newAdmin; 255: } 259 function setNewBathTokenImplementation(address newImplementation) external onlyAdmin { 260 newBathTokenImplementation = newImplementation; 261: } 269 function setPermissionedStrategists(bool _new) external onlyAdmin { 270 permissionedStrategists = _new; 271: } 274 function setCancelTimeDelay(uint256 value) external onlyAdmin { 275 timeDelay = value; 276: } 279 function setReserveRatio(uint256 rr) external onlyAdmin { 280 require(rr <= 100); 281 require(rr > 0); 282 reserveRatio = rr; 283: } 335 function setMarket(address newMarket) external onlyAdmin { 336 RubiconMarketAddress = newMarket; 337: }
File: contracts/rubiconPools/BathToken.sol 245 function setMarket(address newRubiconMarket) external onlyBathHouse { 246 RubiconMarketAddress = newRubiconMarket; 247: } 250 function setBathHouse(address newBathHouse) external onlyBathHouse { 251 bathHouse = newBathHouse; 252: } 260 function setFeeBPS(uint256 _feeBPS) external onlyBathHouse { 261 feeBPS = _feeBPS; 262: }
Use a solidity version of at least 0.8.13 to get the ability to use using for with a list of free functions
There are 2 instances of this issue:
File: contracts/rubiconPools/BathPair.sol #1 8: pragma solidity =0.7.6;
File: contracts/peripheral_contracts/BathBuddy.sol #2 2: pragma solidity >=0.6.0 <0.8.0;
Use a solidity version of at least 0.8.4 to get bytes.concat() instead of abi.encodePacked(<bytes>,<bytes>)
Use a solidity version of at least 0.8.12 to get string.concat() instead of abi.encodePacked(<str>,<str>)
There are 3 instances of this issue:
File: contracts/RubiconMarket.sol #1 7: pragma solidity =0.7.6;
File: contracts/rubiconPools/BathToken.sol #2 8: pragma solidity =0.7.6;
File: contracts/RubiconRouter.sol #3 5: pragma solidity =0.7.6;
1e18) rather than exponentiation (e.g. 10**18)There are 10 instances of this issue:
File: contracts/RubiconMarket.sol 73: uint256 constant WAD = 10**18; 74: uint256 constant RAY = 10**27; 857: pay_amt * 10**9, 859: ) / 10**9; 898: buy_amt * 10**9, 900: ) / 10**9 927: pay_amt * 10**9, 929: ) / 10**9 951: buy_amt * 10**9, 953: ) / 10**9
Some lines use // x and some use //x. The instances below point out the usages that don't follow the majority, within each file
There are 127 instances of this issue:
File: contracts/RubiconMarket.sol 188: /// @dev The mapping that makes up the core orderbook of the exchange 193: /// @dev This parameter is in basis points 196: /// @dev This parameter provides the address to which fees are sent 208: /// @notice Modifier that insures an order exists and is properly in the orderbook 214: /// @notice Modifier that checks the user to make sure they own the offer and its valid before they attempt to cancel it 254: /// @notice Below are the main public entrypoints 270: /// @notice Accept a given `quantity` of an offer. Transfers funds from caller/taker to offer maker, and from market to caller/taker. 271: /// @notice The fee for taker trades is paid in this function. 295: // Fee logic added on taker trades 349: /// @notice Allows the caller to cancel the offer if it is their own. 350: /// @notice This function refunds the offer to the maker. 391: /// @notice Key function to make a new offer. Takes funds from the caller into market escrow. 440: // Fee logic 451: /// @dev After close_time has been reached, no new offers are allowed. 457: /// @dev After close, no new buys are allowed. 464: /// @dev After close, anyone can cancel an offer. 529: /// @dev Below is variable to allow for a proxy-friendly constructor 532: /// @dev unused deprecated variable for applying a token distribution on top of a trade 534: /// @dev unused deprecated variable for applying a token distribution of this token on top of a trade 548: uint256 public dustId; // id of the latest offer marked as dust 550: /// @dev Proxy-safe initialization of storage 555: /// @notice The market fee recipient 561: /// @notice The starting fee on taker trades in basis points 569: // After close, anyone can cancel an offer 579: // ---- Public entrypoints ---- // 598: // Make a new offer. Takes funds from the caller into market escrow. 600: // If matching is enabled: 601: // * creates new offer without putting it in 602: // the sorted list. 603: // * available to authorized contracts only! 604: // * keepers should call insert(id,pos) 605: // to put offer in the sorted list. 607: // If matching is disabled: 608: // * calls expiring market's offer(). 609: // * available to everyone without authorization. 610: // * no sorting is done. 626: // Make a new offer. Takes funds from the caller into market escrow. 677: // Cancel an offer. Refunds offer maker. 712: // Function should be called by keepers. 726: // Function is used to avoid "dust offers" that have 727: // very small amount of tokens to sell, and it would 728: // cost more gas to accept the offer, than the value 729: // of tokens received. 754: // If matchingEnabled true(default), then inserted offers are matched. 755: // Except the ones inserted by contracts, because those end up 756: // in the unsorted list of offers, that must be later sorted by 757: // keepers using insert(). 758: // If matchingEnabled is false then RubiconMarket is reverted to ExpiringMarket, 759: // and matching is not done, and sorted lists are disabled. 771: // the best offer is the lowest one if it's an ask, 772: // and highest one if it's a bid offer 782: // the worse offer is the higher one if its an ask, 783: // a lower one if its a bid offer, 784: // and in both cases the newer one if they're equal. 790: // the better offer is in the lower priced one if its an ask, 791: // the next higher priced one if its a bid offer 792: // and in both cases the older one if they're equal. 807: // Contracts can't calculate the insertion position of their offer because it is not an O(1) operation. 808: // Their offers get put in the unsorted list of offers. 809: // Keepers can calculate the insertion position offchain and pass it to the insert() function to insert 810: // the unsorted offer into the sorted list. Unsorted offers will not be matched, but can be bought with buy(). 816: // Can be used to cycle through all the unsorted offers. 842: // There is a chance that pay_amt is smaller than 1 wei of the other token 855: // if lower 881: // There is a chance that buy_amt is smaller than 1 wei of the other token 957: // ---- Internal Functions ---- // 972: // If offer has become dust during buy, we cancel it 992: // Find the larger-than-id order whose successor is less-than-id. 1004: // Look for an active order. 1013: // if we did find a nearby active offer 1014: // Walk the order book down from there... 1018: // Guaranteed to run at least once because of 1019: // the prior if statements. 1026: // ...or walk it up. 1062: // there is at least one offer stored for token pair 1068: // Ugly hack to work around rounding errors. Based on the idea that 1069: // the furthest the amounts can stray from their "true" values is 1. 1070: // Ergo the worst case has t_pay_amt and m_pay_amt at +1 away from 1071: // their "correct" values and m_buy_amt and t_buy_amt at -1. 1072: // Since (c - 1) * (d - 1) > (a + 1) * (b + 1) is equivalent to 1073: // c * d > a * b + a + b + c + d, we write... 1085: // ^ The `rounding` parameter is a compromise borne of a couple days 1086: // of discussion. 1110: // Make a new offer without putting it in the sorted list. 1111: // Takes funds from the caller into market escrow. 1112: // Keepers should call insert(id,pos) to put offer in the sorted list. 1169: // Remove offer from the sorted list (does not cancel offer) 1183: // offers[id] is not the highest offer 1236: /// @dev unused deprecated function for applying a token distribution on top of a trade 1246: /// @dev unused deprecated variable for applying a token distribution on top of a trade
File: contracts/rubiconPools/BathPair.sol 119: address _bathHouse = msg.sender; //Assume the initializer is BathHouse 224: order memory offer1 = getOfferInfo(info.askId); //ask 225: order memory offer2 = getOfferInfo(info.bidId); //bid 494: uint256 assetRebalAmt, //amount of ASSET in the quote buffer 495: uint256 quoteRebalAmt, //amount of QUOTE in the asset buffer 540: uint256 amount, //fill amount to handle 541: uint256 hurdle, //must clear this on tail off
File: contracts/rubiconPools/BathToken.sol 193: bathHouse = msg.sender; //NOTE: assumed admin is creator on BathHouse 217: feeTo = address(this); //This contract is the fee recipient, rewarding HODLers 218: feeBPS = 3; //Fee set to 3 BPS initially
File: contracts/RubiconRouter.sol 72: //1. Get best offer for each asset 84: //2. Iterate from that offer down the book until topNOrders 124: //3. Return those topNOrders for either side of the order book 133: uint256, //id 165: uint256 expectedMarketFeeBPS //20 198: uint256 expectedMarketFeeBPS //20 200: //**User must approve this contract first** 201: //transfer needed amount here first 243: 0 //naively assume no fill_amt here for loop purposes? 272: //swaps msg.sender entire balance in the trade 282: maxAmount.sub(buy_amt_min.mul(expectedMarketFeeBPS).div(10000)), //account for fee 295: //swaps msg.sender's entire balance in the trade 312: //swaps msg.sender entire balance in the trade 366: ERC20(pay_gem).transferFrom(msg.sender, address(this), max_fill_amount); //transfer pay here 384: uint256 pay_amt, //maker (ask) sell how much 386: uint256 buy_amt, //maker (ask) buy how much 387: ERC20 buy_gem, //maker (ask) buy which token 388: uint256 pos //position to insert offer, 0 should be used if unknown 405: //return any potential fill amount on the offer 413: uint256 pay_amt, //maker (ask) sell how much 414: ERC20 pay_gem, //maker (ask) sell which token 415: uint256 buy_amt, //maker (ask) buy how much 417: uint256 pos //position to insert offer, 0 should be used if unknown 431: //return any potential fill amount on the offer as native ETH 470: //Send back bathTokens to sender 490: //Send back withdrawn native eth to sender 529: //Transfer tokens here first and account for fee
const/immutable variablesIf the variable needs to be different based on which class it comes from, a view/pure function should be used instead (e.g. like this).
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 66: bytes32 public DOMAIN_SEPARATOR;
There are 17 instances of this issue:
File: contracts/RubiconMarket.sol /// @audit blocknumber 540: uint256 delb; //the blocknumber where this entry was marked for delete /// @audit acumulator 851: fill_amt = add(fill_amt, offers[offerId].pay_amt); //Add amount bought to acumulator /// @audit acumulator 860: fill_amt = add(fill_amt, baux); //Add amount bought to acumulator /// @audit acumulator 890: fill_amt = add(fill_amt, offers[offerId].buy_amt); //Add amount sold to acumulator /// @audit acumulator 901: ); //Add amount sold to acumulator
File: contracts/rubiconPools/BathHouse.sol /// @audit liquity 40: /// @notice Key, system-wide risk parameter for all liquity Pools /// @audit gaurentee 132: /// @notice Please note, creating a Bath Token in this fashion ~does not~ gaurentee markets will be made for the new pair. This function signals the desire to have a new pair supported on Rubicon for strategists to consider market-making for /// @audit withdrawls 414: // Note, the option of a fee recipient for pool withdrawls exists. For all pools this is set to the pool itself in production and is visible via ~feeTo~ on any respective contract
File: contracts/rubiconPools/BathPair.sol /// @audit StrategitTrade 81: /// @notice Log a new market-making trade placed by a strategist, resulting in a StrategitTrade /// @audit acitve 211: /// @dev The local array of strategist IDs that exists for any given strategist [query via getOutstandingStrategistTrades()] acts as an acitve RAM for outstanding strategist trades
File: contracts/rubiconPools/BathToken.sol /// @audit intially 29: /// @notice The RubiconMarket.sol instance that all pool liquidity is intially directed to as market-making offers /// @audit constract 220: // Complete constract instantiation /// @audit distibute 627: /// @notice Function to distibute non-underlyingToken Bath Token incentives to pool withdrawers /// @audit releaseable 641: // The BathBuddy pool should accrue ERC-20 rewards just like OZ VestingWallet and simply just release the withdrawer's relative share of releaseable() tokens
File: contracts/peripheral_contracts/BathBuddy.sol /// @audit customizable 17: * The vesting schedule is customizable through the {vestedAmount} function. /// @audit transfering 110: // Keep tokens here by not transfering the _fee anywhere, it is accrued to the Bath Token's Bath Buddy
File: contracts/RubiconRouter.sol /// @audit wouldbe 187: // Return the wouldbe resulting swap amount
There is 1 instance of this issue:
File: contracts/rubiconPools/BathHouse.sol #1 /// @audit Missing: '@return' 389 /// @param underlyingERC20 The underlying ERC-20 asset that underlies the newBathTokenAddress 390 /// @param _feeAdmin Recipient of pool withdrawal fees, typically the pool itself 391 function _createBathToken(ERC20 underlyingERC20, address _feeAdmin) 392 internal 393: returns (address newBathTokenAddress)
indexed fieldsEach event should use three indexed fields if there are three or more fields
There are 33 instances of this issue:
File: contracts/RubiconMarket.sol 115: event LogItemUpdate(uint256 id); 116 event LogTrade( 117 uint256 pay_amt, 118 address indexed pay_gem, 119 uint256 buy_amt, 120 address indexed buy_gem 121: ); 168: event LogInt(string lol, uint256 input); 180: event OfferDeleted(uint256 id); 485 event LogNote( 486 bytes4 indexed sig, 487 address indexed guy, 488 bytes32 indexed foo, 489 bytes32 indexed bar, 490 uint256 wad, 491 bytes fax 492: ) anonymous; 512: event LogBuyEnabled(bool isEnabled); 513: event LogMinSell(address pay_gem, uint256 min_amount); 514: event LogMatchingEnabled(bool isEnabled); 515: event LogUnsortedOffer(uint256 id); 516: event LogSortedOffer(uint256 id); 517: event LogInsert(address keeper, uint256 id); 518: event LogDelete(address keeper, uint256 id); 519: event LogMatch(uint256 id, uint256 amount);
File: contracts/rubiconPools/BathHouse.sol 65 event LogNewBathToken( 66 address underlyingToken, 67 address bathTokenAddress, 68 address bathTokenFeeAdmin, 69 uint256 timestamp, 70 address bathTokenCreator 71: ); 74 event LogOpenCreationSignal( 75 ERC20 newERC20Underlying, 76 address spawnedBathToken, 77 uint256 initialNewBathTokenDeposit, 78 ERC20 pairedExistingAsset, 79 address pairedExistingBathToken, 80 uint256 pairedBathTokenDeposit, 81 address signaler 82: );
File: contracts/rubiconPools/BathPair.sol 82 event LogStrategistTrade( 83 uint256 strategistTradeID, 84 bytes32 askId, 85 bytes32 bidId, 86 address askAsset, 87 address bidAsset, 88 uint256 timestamp, 89 address strategist 90: ); 93 event LogScrubbedStratTrade( 94 uint256 strategistIDScrubbed, 95 uint256 assetFill, 96 address assetAddress, 97 address bathAssetAddress, 98 uint256 quoteFill, 99 address quoteAddress, 100 address bathQuoteAddress 101: ); 104 event LogStrategistRewardClaim( 105 address strategist, 106 address asset, 107 uint256 amountOfReward, 108 uint256 timestamp 109: );
File: contracts/rubiconPools/BathToken.sol 85 event Approval( 86 address indexed owner, 87 address indexed spender, 88 uint256 value 89: ); 92: event Transfer(address indexed from, address indexed to, uint256 value); 95: event LogInit(uint256 timeOfInit); 98 event LogDeposit( 99 uint256 depositedAmt, 100 IERC20 asset, 101 uint256 sharesReceived, 102 address depositor, 103 uint256 underlyingBalance, 104 uint256 outstandingAmount, 105 uint256 totalSupply 106: ); 109 event LogWithdraw( 110 uint256 amountWithdrawn, 111 IERC20 asset, 112 uint256 sharesWithdrawn, 113 address withdrawer, 114 uint256 fee, 115 address feeTo, 116 uint256 underlyingBalance, 117 uint256 outstandingAmount, 118 uint256 totalSupply 119: ); 122 event LogRebalance( 123 IERC20 pool_asset, 124 address destination, 125 IERC20 transferAsset, 126 uint256 rebalAmt, 127 uint256 stratReward, 128 uint256 underlyingBalance, 129 uint256 outstandingAmount, 130 uint256 totalSupply 131: ); 134 event LogPoolCancel( 135 uint256 orderId, 136 IERC20 pool_asset, 137 uint256 outstandingAmountToCancel, 138 uint256 underlyingBalance, 139 uint256 outstandingAmount, 140 uint256 totalSupply 141: ); 144 event LogPoolOffer( 145 uint256 id, 146 IERC20 pool_asset, 147 uint256 underlyingBalance, 148 uint256 outstandingAmount, 149 uint256 totalSupply 150: ); 153 event LogRemoveFilledTradeAmount( 154 IERC20 pool_asset, 155 uint256 fillAmount, 156 uint256 underlyingBalance, 157 uint256 outstandingAmount, 158 uint256 totalSupply 159: ); 162 event Deposit( 163 address indexed caller, 164 address indexed owner, 165 uint256 assets, 166 uint256 shares 167: );
File: contracts/peripheral_contracts/BathBuddy.sol 53: event EtherReleased(uint256 amount); 54: event ERC20Released(address indexed token, uint256 amount); 57 event LogClaimBonusToken( 58 address indexed receiver, 59 address indexed callingPool, 60 uint256 amountReceived, 61 uint256 shares, 62 IERC20 bonusToken, 63 uint256 releasableAmountToWholePool 64: );
File: contracts/RubiconRouter.sol 25: event LogNote(string, uint256); 27 event LogSwap( 28 uint256 inputAmount, 29 address inputERC20, 30 uint256 hurdleBuyAmtMin, 31 address targetERC20, 32 bytes32 indexed pair, 33 uint256 realizedFill, 34 address recipient 35: );
Consider changing the variable to be an unnamed one
There are 15 instances of this issue:
File: contracts/RubiconMarket.sol /// @audit z 57: function min(uint256 x, uint256 y) internal pure returns (uint256 z) { /// @audit z 61: function max(uint256 x, uint256 y) internal pure returns (uint256 z) { /// @audit z 65: function imin(int256 x, int256 y) internal pure returns (int256 z) { /// @audit z 69: function imax(int256 x, int256 y) internal pure returns (int256 z) { /// @audit active 232: function isActive(uint256 id) public view returns (bool active) { /// @audit owner 236: function getOwner(uint256 id) public view returns (address owner) { /// @audit id 382 function make( 383 ERC20 pay_gem, 384 ERC20 buy_gem, 385 uint128 pay_amt, 386 uint128 buy_amt 387: ) external virtual returns (bytes32 id) { /// @audit closed 471: function isClosed() public pure returns (bool closed) { /// @audit success 678 function cancel(uint256 id) 679 public 680 override 681 can_cancel(id) 682: returns (bool success)
File: contracts/rubiconPools/BathToken.sol /// @audit amountWithdrawn 375 function withdraw(uint256 _shares) 376 external 377: returns (uint256 amountWithdrawn) /// @audit totalManagedAssets 388: function totalAssets() public view returns (uint256 totalManagedAssets) { /// @audit shares 435: function deposit(uint256 assets) public returns (uint256 shares) { /// @audit shares 441 function deposit(uint256 assets, address receiver) 442 public 443: returns (uint256 shares) /// @audit maxShares 525: function maxRedeem(address owner) public view returns (uint256 maxShares) {
File: contracts/RubiconRouter.sol /// @audit fill_amt 161 function getExpectedSwapFill( 162 uint256 pay_amt, 163 uint256 buy_amt_min, 164 address[] calldata route, // First address is what is being payed, Last address is what is being bought 165 uint256 expectedMarketFeeBPS //20 166: ) public view returns (uint256 fill_amt) {
🌟 Selected for report: IllIllI
Also found by: 0x1f8b, 0x4non, 0xDjango, 0xNazgul, 0xf15ers, 0xkatana, Chom, DavidGialdi, Dravee, ElKu, FSchmoede, Fitraldys, Funen, GimelSec, JC, Kaiziron, MaratCerby, Metatron, MiloTruck, Picodes, Randyyy, RoiEvenHaim, SmartSek, Tomio, UnusualTurtle, WatchPug, Waze, _Adam, antonttc, asutorufos, berndartmueller, blackscale, blockdev, c3phas, catchup, csanuragjain, defsec, delfin454000, ellahi, fatherOfBlocks, gzeon, hansfriese, ilan, joestakey, minhquanym, oyc_109, pauliax, pedroais, reassor, rfa, rotcivegaf, sach1r0, samruna, sashik_eth, simon135, z3s
301.7788 USDC - $301.78
| Issue | Instances | |
|---|---|---|
| 1 | Multiple address mappings can be combined into a single mapping of an address to a struct, where appropriate | 2 |
| 2 | State variables only set in the constructor should be declared immutable | 3 |
| 3 | State variables can be packed into fewer storage slots | 3 |
| 4 | Using calldata instead of memory for read-only arguments in external functions saves gas | 12 |
| 5 | Avoid contract existence checks by using solidity version 0.8.10 or later | 117 |
| 6 | State variables should be cached in stack variables rather than re-reading them from storage | 71 |
| 7 | Multiple accesses of a mapping should use a local variable cache | 59 |
| 8 | internal functions only called once can be inlined to save gas | 12 |
| 9 | <array>.length should not be looked up in every loop of a for-loop | 3 |
| 10 | require()/revert() strings longer than 32 bytes cost extra gas | 26 |
| 11 | keccak256() should only need to be called on a specific string literal once | 1 |
| 12 | Using bools for storage incurs overhead | 12 |
| 13 | Use a more recent version of solidity | 5 |
| 14 | Use a more recent version of solidity | 1 |
| 15 | Using > 0 costs more gas than != 0 when used on a uint in a require() statement | 6 |
| 16 | It costs more gas to initialize variables to zero than to let the default of zero be applied | 13 |
| 17 | internal functions not called by the contract should be removed to save deployment gas | 7 |
| 18 | ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too) | 8 |
| 19 | Splitting require() statements that use && saves gas | 8 |
| 20 | Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead | 37 |
| 21 | Using private rather than public for constants, saves gas | 1 |
| 22 | Don't compare boolean expressions to boolean literals | 4 |
| 23 | Duplicated require()/revert() checks should be refactored to a modifier or function | 12 |
| 24 | Division by two should use bit shifting | 4 |
| 25 | require() or revert() statements that check input arguments should be at the top of the function | 4 |
| 26 | Empty blocks should be removed or emit something | 3 |
| 27 | Superfluous event fields | 8 |
| 28 | Functions guaranteed to revert when called by normal users can be marked payable | 35 |
Total: 477 instances over 28 issues
address mappings can be combined into a single mapping of an address to a struct, where appropriateSaves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.
There are 2 instances of this issue:
File: contracts/RubiconMarket.sol #1 543 mapping(address => mapping(address => uint256)) public _best; //id of the highest offer for a token pair 544 mapping(address => mapping(address => uint256)) public _span; //number of offers stored for token pair in sorted orderbook 545: mapping(address => uint256) public _dust; //minimum sell amount for a token to avoid dust offers
File: contracts/rubiconPools/BathPair.sol #2 52 mapping(address => mapping(address => mapping(address => uint256[]))) 53 public outOffersByStrategist; 54 55 /// @notice Tracks the market-kaing fill amounts on a per-asset basis of a strategist 56 /// @dev strategist => erc20asset => fill amount per asset; 57: mapping(address => mapping(address => uint256)) public strategist2Fills;
immutableAvoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).
There are 3 instances of this issue:
File: contracts/peripheral_contracts/BathBuddy.sol #1 31: address public beneficiary;
File: contracts/peripheral_contracts/BathBuddy.sol #2 32: uint64 public start;
File: contracts/peripheral_contracts/BathBuddy.sol #3 33: uint64 public duration;
If variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables can also be cheaper
There are 3 instances of this issue:
File: contracts/RubiconMarket.sol #1 /// @audit Variable ordering with 4 slots instead of the current 5: /// @audit uint256(32):last_offer_id, mapping(32):offers, uint256(32):feeBPS, address(20):feeTo, bool(1):locked 186: uint256 public last_offer_id;
File: contracts/rubiconPools/BathHouse.sol #2 /// @audit Variable ordering with 10 slots instead of the current 11: /// @audit string(32):name, mapping(32):approvedStrategists, uint256(32):reserveRatio, uint256(32):timeDelay, mapping(32):tokenToBathToken, address(20):admin, bool(1):initialized, bool(1):permissionedStrategists, uint8(1):bpsToStrategists, address(20):proxyManager, address(20):RubiconMarketAddress, address(20):approvedPairContract, address(20):newBathTokenImplementation 20: string public name;
File: contracts/rubiconPools/BathToken.sol #3 /// @audit Variable ordering with 17 slots instead of the current 18: /// @audit string(32):symbol, string(32):name, uint256(32):feeBPS, uint256(32):totalSupply, uint256(32):outstandingAmount, uint256[](32):deprecatedStorageArray, mapping(32):deprecatedMapping, mapping(32):balanceOf, mapping(32):allowance, bytes32(32):DOMAIN_SEPARATOR, mapping(32):nonces, address[](32):bonusTokens, address(20):RubiconMarketAddress, bool(1):initialized, uint8(1):decimals, address(20):bathHouse, address(20):feeTo, user-defined(20):underlyingToken, user-defined(20):rewardsVestingWallet 22: bool public initialized;
calldata instead of memory for read-only arguments in external functions saves gasWhen a function with a memory array is called externally, the abi.decode() step has to use a for-loop to copy each index of the calldata to the memory index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length). Using calldata directly, obliviates the need for such a loop in the contract code and runtime execution.
If the array is passed to an internal function which passes the array to another internal function where the array is modified and therefore memory is used in the external call, it's still more gass-efficient to use calldata when the external function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one
There are 12 instances of this issue:
File: contracts/rubiconPools/BathPair.sol 413: address[2] memory tokenPair, // ASSET, Then Quote 414: uint256[] memory askNumerators, // Quote / Asset 415: uint256[] memory askDenominators, // Asset / Quote 416: uint256[] memory bidNumerators, // size in ASSET 417: uint256[] memory bidDenominators // size in QUOTES 464: uint256[] memory ids, 465: address[2] memory tokenPair, // ASSET, Then Quote 466: uint256[] memory askNumerators, // Quote / Asset 467: uint256[] memory askDenominators, // Asset / Quote 468: uint256[] memory bidNumerators, // size in ASSET 469: uint256[] memory bidDenominators // size in QUOTES 578: function scrubStrategistTrades(uint256[] memory ids)
Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (700 gas), to check for contract existence for external calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value
There are 117 instances of this issue:
File: contracts/RubiconMarket.sol 298: _offer.buy_gem.transferFrom(msg.sender, feeTo, fee), 305: _offer.buy_gem.transferFrom(msg.sender, _offer.owner, spend), 309: _offer.pay_gem.transfer(msg.sender, quantity), 361: require(_offer.pay_gem.transfer(_offer.owner, _offer.pay_amt)); 416: require(pay_gem.transferFrom(msg.sender, address(this), pay_amt)); 665 IAqueduct(AqueductAddress).distributeToMakerAndTaker( 666 getOwner(id), 667 msg.sender 668: );
File: contracts/rubiconPools/BathHouse.sol 157 newBathTokenUnderlying.transferFrom( 158 msg.sender, 159 address(this), 160 initialLiquidityNew 161: ), 165: newBathTokenUnderlying.approve(newOne, initialLiquidityNew); 168: IBathToken(newOne).deposit(initialLiquidityNew, msg.sender); 172 desiredPairedAsset.transferFrom( 173 msg.sender, 174 address(this), 175 initialLiquidityExistingBathToken 176: ), 180 desiredPairedAsset.approve( 181 pairedPool, 182 initialLiquidityExistingBathToken 183: ); 186 IBathToken(pairedPool).deposit( 187 initialLiquidityExistingBathToken, 188 msg.sender 189: ); 242: IBathPair(_bathPairAddress).initialized() != true,
File: contracts/rubiconPools/BathPair.sol 121: IBathHouse(_bathHouse).getMarket() != 123: IBathHouse(_bathHouse).initialized(), 128: RubiconMarketAddress = IBathHouse(_bathHouse).getMarket(); 149: IBathHouse(bathHouse).isApprovedStrategist(targetStrategist) == 168 bathAssetAddress = IBathHouse(bathHouse).tokenToBathToken( 169 underlyingAsset 170: ); 171 bathQuoteAddress = IBathHouse(bathHouse).tokenToBathToken( 172 underlyingQuote 173: ); 176: IBathToken(bathAssetAddress).underlyingBalance().mul( 177: IBathHouse(bathHouse).reserveRatio() 179: ).div(100) <= IERC20(underlyingAsset).balanceOf(bathAssetAddress), 184: IBathToken(bathQuoteAddress).underlyingBalance().mul( 185: IBathHouse(bathHouse).reserveRatio() 187: ).div(100) <= IERC20(underlyingQuote).balanceOf(bathQuoteAddress), 218 address bathAssetAddress = IBathHouse(bathHouse).tokenToBathToken( 219 _asset 220: ); 221 address bathQuoteAddress = IBathHouse(bathHouse).tokenToBathToken( 222 _quote 223: ); 298: ) = IRubiconMarket(RubiconMarketAddress).getOffer(id); 367 uint256 newAskID = IBathToken(bathAssetAddress).placeOffer( 368 ask.pay_amt, 369 ask.pay_gem, 370 ask.buy_amt, 371 ask.buy_gem 372: ); 374 uint256 newBidID = IBathToken(bathQuoteAddress).placeOffer( 375 bid.pay_amt, 376 bid.pay_gem, 377 bid.buy_amt, 378 bid.buy_gem 379: ); 500 address _bathAssetAddress = IBathHouse(_bathHouse).tokenToBathToken( 501 _underlyingAsset 502: ); 503 address _bathQuoteAddress = IBathHouse(_bathHouse).tokenToBathToken( 504 _underlyingQuote 505: ); 512: uint16 stratReward = IBathHouse(_bathHouse).getBPSToStrats(); 545: uint16 stratRewardBPS = IBathHouse(bathHouse).getBPSToStrats(); 555 IStrategistUtility(_stratUtil).UNIdump( 556 amount.sub((stratRewardBPS.mul(amount)).div(10000)), 557 tokenToHandle, 558 targetToken, 559 hurdle, 560 _poolFee, 561 targetPool 562: ); 599: fillCountA.mul(IERC20(asset).balanceOf(address(this))) 601: IERC20(asset).transfer(msg.sender, booty); 613: fillCountQ.mul(IERC20(quote).balanceOf(address(this))) 615: IERC20(quote).transfer(msg.sender, booty);
File: contracts/rubiconPools/BathToken.sol 188: abi.encodePacked(("bath"), token.symbol()) 211: decimals = token.decimals(); // v1 Change - 4626 Adherence 214: IERC20(address(token)).approve(RubiconMarketAddress, 2**256 - 1); 228: IBathHouse(bathHouse).isApprovedPair(msg.sender) == true, 256: underlyingToken.approve(RubiconMarketAddress, 2**256 - 1); 323 uint256 id = IRubiconMarket(RubiconMarketAddress).offer( 324 pay_amt, 325 pay_gem, 326 buy_amt, 327 buy_gem, 328 0, 329 false 330: ); 353 IERC20(filledAssetToRebalance).transfer( 354 destination, 355 rebalAmt.sub(stratReward) 356: ); 357: IERC20(filledAssetToRebalance).transfer(msg.sender, stratReward); 562: uint256 _before = underlyingToken.balanceOf(address(this)); 565: underlyingToken.transferFrom(msg.sender, address(this), assets); 566: uint256 _after = underlyingToken.balanceOf(address(this)); 602: underlyingToken.transfer(feeTo, _fee); 605: underlyingToken.transfer(receiver, amountWithdrawn); 757: uint256 _pool = IERC20(underlyingToken).balanceOf(address(this));
File: contracts/peripheral_contracts/BathBuddy.sol 114: token.transfer(recipient, amountWithdrawn); 140: IERC20(token).balanceOf(address(this)) + released(token),
File: contracts/RubiconRouter.sol 73 uint256 bestAskID = RubiconMarket(_RubiconMarketAddress).getBestOffer( 74 asset, 75 quote 76: ); 77 uint256 bestBidID = RubiconMarket(_RubiconMarketAddress).getBestOffer( 78 quote, 79 asset 80: ); 95: ) = RubiconMarket(_RubiconMarketAddress).getOffer(bestAskID); 101: ) = RubiconMarket(_RubiconMarketAddress).getOffer(bestBidID); 106 uint256 nextBestAsk = RubiconMarket(_RubiconMarketAddress) 107: .getWorseOffer(lastAsk); 108 uint256 nextBestBid = RubiconMarket(_RubiconMarketAddress) 109: .getWorseOffer(lastBid); 110 (uint256 ask_pay_amt, , uint256 ask_buy_amt, ) = RubiconMarket( 111 _RubiconMarketAddress 112: ).getOffer(nextBestAsk); 113 (uint256 bid_pay_amt, , uint256 bid_buy_amt, ) = RubiconMarket( 114 _RubiconMarketAddress 115: ).getOffer(nextBestBid); 141 uint256 offer = RubiconMarket(_market).getBestOffer( 142 ERC20(asset), 143 ERC20(quote) 144: ); 150: ) = RubiconMarket(_market).getOffer(offer); 157: ERC20(toApprove).approve(RubiconMarketAddress, 2**256 - 1); 178 uint256 wouldBeFillAmount = RubiconMarket(_market).getBuyAmount( 179 ERC20(output), 180 ERC20(input), 181 _pay 182: ); 202 ERC20(route[0]).transferFrom( 203 msg.sender, 204 address(this), 205 pay_amt.add(pay_amt.mul(expectedMarketFeeBPS).div(10000)) // Account for expected fee 206: ); 236: if (ERC20(input).allowance(address(this), _market) == 0) { 239 uint256 fillAmount = RubiconMarket(_market).sellAllAmount( 240 ERC20(input), 241 _pay, 242 ERC20(output), 243 0 //naively assume no fill_amt here for loop purposes? 244: ); 251: ERC20(route[route.length - 1]).transfer(to, currentAmount); 273: uint256 maxAmount = ERC20(route[0]).balanceOf(msg.sender); 274 ERC20(route[0]).transferFrom( 275 msg.sender, 276 address(this), 277 maxAmount // Account for expected fee 278: ); 296: uint256 maxAmount = ERC20(buy_gem).balanceOf(msg.sender); 297 fill = RubiconMarket(RubiconMarketAddress).buyAllAmount( 298 buy_gem, 299 maxAmount, 300 pay_gem, 301 max_fill_amount 302: ); 303: ERC20(buy_gem).transfer(msg.sender, fill); 313: uint256 maxAmount = ERC20(buy_gem).balanceOf(msg.sender); 314 fill = RubiconMarket(RubiconMarketAddress).sellAllAmount( 315 pay_gem, 316 maxAmount, 317 buy_gem, 318 min_fill_amount 319: ); 320: ERC20(buy_gem).transfer(msg.sender, fill); 332: uint256 _before = ERC20(_weth).balanceOf(address(this)); 342 fill = RubiconMarket(RubiconMarketAddress).buyAllAmount( 343 buy_gem, 344 buy_amt, 345 ERC20(wethAddress), 346 max_fill_amount 347: ); 348: ERC20(buy_gem).transfer(msg.sender, buy_amt); 350: uint256 _after = ERC20(_weth).balanceOf(address(this)); 355: WETH9(wethAddress).withdraw(delta); 356: msg.sender.transfer(delta); 366: ERC20(pay_gem).transferFrom(msg.sender, address(this), max_fill_amount); //transfer pay here 367 fill = RubiconMarket(RubiconMarketAddress).buyAllAmount( 368 ERC20(wethAddress), 369 buy_amt, 370 pay_gem, 371 max_fill_amount 372: ); 373: WETH9(wethAddress).withdraw(buy_amt); // Fill in WETH 374: msg.sender.transfer(buy_amt); // Return native ETH 377: ERC20(pay_gem).transfer(msg.sender, max_fill_amount - fill); 394: uint256 _before = ERC20(buy_gem).balanceOf(address(this)); 396 uint256 id = RubiconMarket(RubiconMarketAddress).offer( 397 pay_amt, 398 ERC20(wethAddress), 399 buy_amt, 400 buy_gem, 401 pos 402: ); 403: uint256 _after = ERC20(buy_gem).balanceOf(address(this)); 406: ERC20(buy_gem).transfer(msg.sender, _after - _before); 419: ERC20(pay_gem).transferFrom(msg.sender, address(this), pay_amt); 421: uint256 _before = ERC20(wethAddress).balanceOf(address(this)); 422 uint256 id = RubiconMarket(RubiconMarketAddress).offer( 423 pay_amt, 424 pay_gem, 425 buy_amt, 426 ERC20(wethAddress), 427 pos 428: ); 429: uint256 _after = ERC20(wethAddress).balanceOf(address(this)); 433: WETH9(wethAddress).withdraw(delta); 434: msg.sender.transfer(delta); 441 (uint256 pay_amt, ERC20 pay_gem, , ) = RubiconMarket( 442 RubiconMarketAddress 443: ).getOffer(id); 450: WETH9(wethAddress).withdraw(pay_amt); 451: msg.sender.transfer(pay_amt); 460: IERC20 target = IBathToken(targetPool).underlyingToken(); 464: if (target.allowance(address(this), targetPool) == 0) { 465: target.approve(targetPool, amount); 469: newShares = IBathToken(targetPool).deposit(amount); 471: ERC20(targetPool).transfer(msg.sender, newShares); 480: IERC20 target = IBathToken(targetPool).underlyingToken(); 483: IBathToken(targetPool).balanceOf(msg.sender) >= shares, 486: IBathToken(targetPool).transferFrom(msg.sender, address(this), shares); 487: withdrawnWETH = IBathToken(targetPool).withdraw(shares); 488: WETH9(wethAddress).withdraw(withdrawnWETH); 491: msg.sender.transfer(withdrawnWETH); 531 ERC20(route[0]).transferFrom( 532 msg.sender, 533 address(this), 534 pay_amt.add(pay_amt.mul(expectedMarketFeeBPS).div(10000)) 535: ), 546: WETH9(wethAddress).withdraw(fill); 548: msg.sender.transfer(fill);
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replace each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
There are 71 instances of this issue:
File: contracts/RubiconMarket.sol /// @audit owner 24: emit LogSetOwner(owner); /// @audit last_offer_id 437: return last_offer_id; /// @audit feeTo 330: feeTo, /// @audit buyEnabled 749: emit LogBuyEnabled(buyEnabled); /// @audit matchingEnabled 766: emit LogMatchingEnabled(matchingEnabled); /// @audit _head 1211: if (_head == id) { /// @audit offers[id].pay_gem 262: offers[id].pay_gem, /// @audit offers[id].buy_gem 263: offers[id].buy_gem, /// @audit _rank[id].delb 718: _rank[id].delb < block.number - 10 /// @audit offers[offerId].buy_amt 849: if (pay_amt >= offers[offerId].buy_amt) { /// @audit offers[offerId].buy_amt 852: pay_amt = sub(pay_amt, offers[offerId].buy_amt); //Decrease amount to sell /// @audit offers[offerId].buy_amt 858: rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId].pay_amt 851: fill_amt = add(fill_amt, offers[offerId].pay_amt); //Add amount bought to acumulator /// @audit offers[offerId].pay_amt 853: take(bytes32(offerId), uint128(offers[offerId].pay_amt)); //We take the whole offer /// @audit offers[offerId].pay_amt 858: rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId].pay_amt 888: if (buy_amt >= offers[offerId].pay_amt) { /// @audit offers[offerId].pay_amt 891: buy_amt = sub(buy_amt, offers[offerId].pay_amt); //Decrease amount to buy /// @audit offers[offerId].pay_amt 892: take(bytes32(offerId), uint128(offers[offerId].pay_amt)); //We take the whole offer /// @audit offers[offerId].pay_amt 899: rdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[offerId].buy_amt 890: fill_amt = add(fill_amt, offers[offerId].buy_amt); //Add amount sold to acumulator /// @audit offers[offerId].buy_amt 899: rdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[offerId].buy_amt 917: pay_amt = sub(pay_amt, offers[offerId].buy_amt); //Decrease amount to pay /// @audit offers[offerId].buy_amt 928: rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId].pay_amt 928: rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId].pay_amt 941: buy_amt = sub(buy_amt, offers[offerId].pay_amt); //Decrease amount to buy /// @audit offers[offerId].pay_amt 952: rdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[offerId].buy_amt 952: rdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[id].pay_amt 975: offers[id].pay_amt < _dust[address(offers[id].pay_gem)]
File: contracts/rubiconPools/BathHouse.sol /// @audit admin 125: approveStrategist(admin); /// @audit newBathTokenImplementation 426: newBathTokenImplementation,
File: contracts/rubiconPools/BathPair.sol /// @audit bathHouse 171: bathQuoteAddress = IBathHouse(bathHouse).tokenToBathToken( /// @audit bathHouse 177: IBathHouse(bathHouse).reserveRatio() /// @audit bathHouse 185: IBathHouse(bathHouse).reserveRatio() /// @audit bathHouse 221: address bathQuoteAddress = IBathHouse(bathHouse).tokenToBathToken( /// @audit last_stratTrade_id 207: return last_stratTrade_id;
File: contracts/rubiconPools/BathToken.sol /// @audit RubiconMarketAddress 214: IERC20(address(token)).approve(RubiconMarketAddress, 2**256 - 1); /// @audit feeTo 602: underlyingToken.transfer(feeTo, _fee); /// @audit feeTo 613: feeTo, /// @audit underlyingToken 565: underlyingToken.transferFrom(msg.sender, address(this), assets); /// @audit underlyingToken 566: uint256 _after = underlyingToken.balanceOf(address(this)); /// @audit underlyingToken 577: underlyingToken, /// @audit underlyingToken 605: underlyingToken.transfer(receiver, amountWithdrawn); /// @audit underlyingToken 609: underlyingToken, /// @audit totalSupply 400: assets.mul(totalSupply) /// @audit totalSupply 459: ).div(totalSupply); /// @audit totalSupply 570: assets.mul(totalSupply) /// @audit totalSupply 582: totalSupply /// @audit totalSupply 616: totalSupply /// @audit outstandingAmount 288: outstandingAmount, /// @audit outstandingAmount 300: outstandingAmount, /// @audit outstandingAmount 337: outstandingAmount, /// @audit bonusTokens 635: for (uint256 index = 0; index < bonusTokens.length; index++) { /// @audit bonusTokens 636: IERC20 token = IERC20(bonusTokens[index]); /// @audit rewardsVestingWallet 643: rewardsVestingWallet.release(
File: contracts/peripheral_contracts/BathBuddy.sol /// @audit start 156: } else if (timestamp > start + duration) { /// @audit start 159: return (totalAllocation * (timestamp - start)) / duration; /// @audit duration 159: return (totalAllocation * (timestamp - start)) / duration;
File: contracts/RubiconRouter.sol /// @audit RubiconMarketAddress 449: outcome = RubiconMarket(RubiconMarketAddress).cancel(id); /// @audit wethAddress 340: WETH9(wethAddress).deposit{value: max_fill_withFee}(); // Pay with native ETH -> WETH /// @audit wethAddress 345: ERC20(wethAddress), /// @audit wethAddress 355: WETH9(wethAddress).withdraw(delta); /// @audit wethAddress 373: WETH9(wethAddress).withdraw(buy_amt); // Fill in WETH /// @audit wethAddress 398: ERC20(wethAddress), /// @audit wethAddress 426: ERC20(wethAddress), /// @audit wethAddress 429: uint256 _after = ERC20(wethAddress).balanceOf(address(this)); /// @audit wethAddress 433: WETH9(wethAddress).withdraw(delta); /// @audit wethAddress 450: WETH9(wethAddress).withdraw(pay_amt); /// @audit wethAddress 468: WETH9(wethAddress).deposit{value: amount}(); /// @audit wethAddress 488: WETH9(wethAddress).withdraw(withdrawnWETH); /// @audit wethAddress 508: WETH9(wethAddress).deposit{value: amtWithFee}(); /// @audit wethAddress 546: WETH9(wethAddress).withdraw(fill);
The instances below point to the second+ access of a value inside a mapping, within a function. Caching a mapping's value in a local storage variable when the value is accessed multiple times, saves ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.
There are 59 instances of this issue:
File: contracts/RubiconMarket.sol /// @audit offers[id] 260: keccak256(abi.encodePacked(offers[id].pay_gem, offers[id].buy_gem)), /// @audit offers[id] 261: offers[id].owner, /// @audit offers[id] 262: offers[id].pay_gem, /// @audit offers[id] 263: offers[id].buy_gem, /// @audit offers[id] 264: uint128(offers[id].pay_amt), /// @audit offers[id] 265: uint128(offers[id].buy_amt), /// @audit offers[id] 266: offers[id].timestamp /// @audit offers[id] 303: offers[id].buy_amt = sub(_offer.buy_amt, spend); /// @audit offers[id] 341: if (offers[id].pay_amt == 0) { /// @audit _rank[id] 718: _rank[id].delb < block.number - 10 /// @audit _rank[id] 824: _rank[id].prev != 0 || /// @audit offers[id] 825: _best[address(offers[id].pay_gem)][address(offers[id].buy_gem)] == /// @audit offers[offerId] 845: wdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[offerId] 849: if (pay_amt >= offers[offerId].buy_amt) { /// @audit offers[offerId] 851: fill_amt = add(fill_amt, offers[offerId].pay_amt); //Add amount bought to acumulator /// @audit offers[offerId] 852: pay_amt = sub(pay_amt, offers[offerId].buy_amt); //Decrease amount to sell /// @audit offers[offerId] 853: take(bytes32(offerId), uint128(offers[offerId].pay_amt)); //We take the whole offer /// @audit offers[offerId] 858: rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId] 858: rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId] 884: wdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId] 888: if (buy_amt >= offers[offerId].pay_amt) { /// @audit offers[offerId] 890: fill_amt = add(fill_amt, offers[offerId].buy_amt); //Add amount sold to acumulator /// @audit offers[offerId] 891: buy_amt = sub(buy_amt, offers[offerId].pay_amt); //Decrease amount to buy /// @audit offers[offerId] 892: take(bytes32(offerId), uint128(offers[offerId].pay_amt)); //We take the whole offer /// @audit offers[offerId] 899: rdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[offerId] 899: rdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[offerId] 916: fill_amt = add(fill_amt, offers[offerId].pay_amt); //Add amount to buy accumulator /// @audit offers[offerId] 917: pay_amt = sub(pay_amt, offers[offerId].buy_amt); //Decrease amount to pay /// @audit offers[offerId] 928: rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId] 928: rdiv(offers[offerId].pay_amt, offers[offerId].buy_amt) /// @audit offers[offerId] 940: fill_amt = add(fill_amt, offers[offerId].buy_amt); //Add amount to pay accumulator /// @audit offers[offerId] 941: buy_amt = sub(buy_amt, offers[offerId].pay_amt); //Decrease amount to buy /// @audit offers[offerId] 952: rdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[offerId] 952: rdiv(offers[offerId].buy_amt, offers[offerId].pay_amt) /// @audit offers[id] 975: offers[id].pay_amt < _dust[address(offers[id].pay_gem)] /// @audit offers[id] 975: offers[id].pay_amt < _dust[address(offers[id].pay_gem)] /// @audit offers[id] 988: address pay_gem = address(offers[id].pay_gem); /// @audit _rank[pos] 1022: pos = _rank[pos].prev; /// @audit _rank[pos] 1029: pos = _rank[pos].next; /// @audit offers[high] 1043: mul(offers[high].buy_amt, offers[low].pay_amt); /// @audit offers[low] 1043: mul(offers[high].buy_amt, offers[low].pay_amt); /// @audit offers[best_maker_id] 1066: m_pay_amt = offers[best_maker_id].pay_amt; /// @audit offers[id] 1134: ERC20 pay_gem = offers[id].pay_gem; /// @audit offers[pos] 1139: offers[pos].buy_gem != buy_gem || /// @audit _rank[pos] 1149: _rank[pos].prev = id; /// @audit _rank[id] 1162: _rank[id].prev = prev_id; /// @audit offers[id] 1174: address pay_gem = address(offers[id].pay_gem); /// @audit _rank[id] 1184: require(_rank[_rank[id].next].prev == id); /// @audit _rank[<etc>] 1185: _rank[_rank[id].next].prev = _rank[id].prev; /// @audit _rank[id] 1185: _rank[_rank[id].next].prev = _rank[id].prev; /// @audit _rank[id] 1185: _rank[_rank[id].next].prev = _rank[id].prev; /// @audit _rank[id] 1188: _best[pay_gem][buy_gem] = _rank[id].prev; /// @audit _rank[id] 1191: if (_rank[id].prev != 0) { /// @audit _rank[<etc>] 1193: require(_rank[_rank[id].prev].next == id); /// @audit _rank[id] 1193: require(_rank[_rank[id].prev].next == id); /// @audit _rank[<etc>] 1194: _rank[_rank[id].prev].next = _rank[id].next; /// @audit _rank[id] 1194: _rank[_rank[id].prev].next = _rank[id].next; /// @audit _rank[id] 1194: _rank[_rank[id].prev].next = _rank[id].next; /// @audit _rank[id] 1198: _rank[id].delb = block.number; //mark _rank[id] for deletion
internal functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
There are 12 instances of this issue:
File: contracts/RubiconMarket.sol 32: function isAuthorized(address src) internal view returns (bool) { 435: function _next_id() internal returns (uint256) { 1001: function _findpos(uint256 id, uint256 pos) internal view returns (uint256) { 1049 function _matcho( 1050 uint256 t_pay_amt, //taker sell how much 1051 ERC20 t_pay_gem, //taker sell which token 1052 uint256 t_buy_amt, //taker buy how much 1053 ERC20 t_buy_gem, //taker buy which token 1054 uint256 pos, //position id 1055 bool rounding //match "close enough" orders? 1056: ) internal returns (uint256 id) {
File: contracts/rubiconPools/BathPair.sol 160 function enforceReserveRatio( 161 address underlyingAsset, 162 address underlyingQuote 163 ) 164 internal 165 view 166: returns (address bathAssetAddress, address bathQuoteAddress) 205: function _next_id() internal returns (uint256) { 213: function handleStratOrderAtID(uint256 id) internal { 305 function getIndexFromElement(uint256 uid, uint256[] storage array) 306 internal 307 view 308: returns (uint256 _index)
File: contracts/rubiconPools/BathToken.sol 629 function distributeBonusTokenRewards( 630 address receiver, 631 uint256 sharesWithdrawn, 632: uint256 initialTotalSupply 657: function _mint(address to, uint256 value) internal { 663: function _burn(address from, uint256 value) internal {
File: contracts/peripheral_contracts/BathBuddy.sol 149 function _vestingSchedule(uint256 totalAllocation, uint64 timestamp) 150 internal 151 view 152: returns (uint256)
<array>.length should not be looked up in every loop of a for-loopThe overheads outlined below are PER LOOP, excluding the first loop
MLOAD (3 gas)CALLDATALOAD (3 gas)Caching the length changes each of these to a DUP<N> (3 gas), and gets rid of the extra DUP<N> needed to store the stack offset
There are 3 instances of this issue:
File: contracts/rubiconPools/BathPair.sol #1 311: for (uint256 index = 0; index < array.length; index++) {
File: contracts/rubiconPools/BathPair.sol #2 582: for (uint256 index = 0; index < ids.length; index++) {
File: contracts/rubiconPools/BathToken.sol #3 635: for (uint256 index = 0; index < bonusTokens.length; index++) {
require()/revert() strings longer than 32 bytes cost extra gasThere are 26 instances of this issue:
File: contracts/RubiconMarket.sol 304 require( 305 _offer.buy_gem.transferFrom(msg.sender, _offer.owner, spend), 306 "_offer.buy_gem.transferFrom(msg.sender, _offer.owner, spend) failed - check that you can pay the fee" 307: ); 308 require( 309 _offer.pay_gem.transfer(msg.sender, quantity), 310 "_offer.pay_gem.transfer(msg.sender, quantity) failed" 311: ); 571: require(isActive(id), "Offer was deleted or taken, or never existed."); 572 require( 573 isClosed() || msg.sender == getOwner(id) || id == dustId, 574 "Offer can not be cancelled because user is not owner, and market is open, and offer sells required amount of tokens." 575: );
File: contracts/rubiconPools/BathHouse.sol 143 require( 144 getBathTokenfromAsset(newBathTokenUnderlying) == address(0), 145 "bathToken already exists for that ERC20" 146: ); 147 require( 148 getBathTokenfromAsset(desiredPairedAsset) != address(0), 149 "bathToken does not exist for that desiredPairedAsset" 150: ); 156 require( 157 newBathTokenUnderlying.transferFrom( 158 msg.sender, 159 address(this), 160 initialLiquidityNew 161 ), 162 "Couldn't transferFrom your initial liquidity - make sure to approve BathHouse.sol" 163: ); 171 require( 172 desiredPairedAsset.transferFrom( 173 msg.sender, 174 address(this), 175 initialLiquidityExistingBathToken 176 ), 177 "Couldn't transferFrom your initial liquidity - make sure to approve BathHouse.sol" 178: ); 397 require( 398 _underlyingERC20 != address(0), 399 "Cant create bathToken for zero address" 400: ); 409 require( 410 newBathTokenImplementation != address(0), 411 "no implementation set for bathTokens" 412: );
File: contracts/rubiconPools/BathPair.sol 148 require( 149 IBathHouse(bathHouse).isApprovedStrategist(targetStrategist) == 150 true, 151 "you are not an approved strategist - bathPair" 152: ); 174 require( 175 ( 176 IBathToken(bathAssetAddress).underlyingBalance().mul( 177 IBathHouse(bathHouse).reserveRatio() 178 ) 179 ).div(100) <= IERC20(underlyingAsset).balanceOf(bathAssetAddress), 180 "Failed to meet asset pool reserve ratio" 181: ); 182 require( 183 ( 184 IBathToken(bathQuoteAddress).underlyingBalance().mul( 185 IBathHouse(bathHouse).reserveRatio() 186 ) 187 ).div(100) <= IERC20(underlyingQuote).balanceOf(bathQuoteAddress), 188 "Failed to meet quote pool reserve ratio" 189: ); 318: require(assigned, "Didnt Find that element in live list, cannot scrub"); 570 require( 571 msg.sender == strategistTrades[id].strategist, 572 "you are not the strategist that made this order" 573: );
File: contracts/rubiconPools/BathToken.sol 235 require( 236 msg.sender == bathHouse, 237 "caller is not bathHouse - BathToken.sol" 238: ); 470: require(_shares == shares, "did not mint expected share count"); 510 require( 511 owner == msg.sender, 512 "This implementation does not support non-sender owners from withdrawing user shares" 513: ); 516 require( 517 assetsReceived >= assets, 518 "You cannot withdraw the amount of assets you expected" 519: ); 547 require( 548 owner == msg.sender, 549 "This implementation does not support non-sender owners from withdrawing user shares" 550: );
File: contracts/peripheral_contracts/BathBuddy.sol 43 require( 44 beneficiaryAddress != address(0), 45 "VestingWallet: beneficiary is zero address" 46: ); 94 require( 95 msg.sender == beneficiary, 96 "Caller is not the Bath Token beneficiary of these rewards" 97: );
File: contracts/RubiconRouter.sol 336 require( 337 msg.value >= max_fill_withFee, 338 "must send as much ETH as max_fill_withFee" 339: ); 390 require( 391 msg.value >= pay_amt, 392 "didnt send enough native ETH for WETH offer" 393: ); 444 require( 445 address(pay_gem) == wethAddress, 446 "trying to cancel a non WETH order" 447: ); 504 require( 505 msg.value >= amtWithFee, 506 "must send enough native ETH to pay as weth and account for fee" 507: );
keccak256() should only need to be called on a specific string literal onceIt should be saved to an immutable variable, and the variable used instead. If the hash is being used as a part of a function selector, the cast to bytes4 should also only be done once
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 201 keccak256( 202 "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)" 203: ),
bools for storage incurs overhead// Booleans are more expensive than uint256 or any type that takes up a full // word because each write operation emits an extra SLOAD to first read the // slot's contents, replace the bits taken up by the boolean, and then write // back. This is the compiler's defense against contract upgrades and // pointer aliasing, and it cannot be disabled.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27
Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from 'false' to 'true', after having been 'true' in the past
There are 12 instances of this issue:
File: contracts/RubiconMarket.sol 191: bool locked; 449: bool public stopped; 526: bool public buyEnabled = true; //buy enabled 527: bool public matchingEnabled = true; //true: enable matching, 530: bool public initialized; 533: bool public AqueductDistributionLive;
File: contracts/rubiconPools/BathHouse.sol 32: mapping(address => bool) public approvedStrategists; 35: bool public initialized; 38: bool public permissionedStrategists;
File: contracts/rubiconPools/BathPair.sol 32: bool public initialized;
File: contracts/rubiconPools/BathToken.sol 22: bool public initialized;
File: contracts/RubiconRouter.sol 23: bool public started;
Use a solidity version of at least 0.8.0 to get overflow protection without SafeMath
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining
Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads
Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings
Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
There are 5 instances of this issue:
File: contracts/RubiconMarket.sol 7: pragma solidity =0.7.6;
File: contracts/rubiconPools/BathHouse.sol 7: pragma solidity =0.7.6;
File: contracts/rubiconPools/BathPair.sol 8: pragma solidity =0.7.6;
File: contracts/rubiconPools/BathToken.sol 8: pragma solidity =0.7.6;
File: contracts/RubiconRouter.sol 5: pragma solidity =0.7.6;
Use a solidity version of at least 0.8.2 to get simple compiler automatic inlining
Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads
Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings
Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value
There is 1 instance of this issue:
File: contracts/peripheral_contracts/BathBuddy.sol #1 2: pragma solidity >=0.6.0 <0.8.0;
> 0 costs more gas than != 0 when used on a uint in a require() statementThis change saves 6 gas per instance
There are 6 instances of this issue:
File: contracts/RubiconMarket.sol 400: require(pay_amt > 0); 402: require(buy_amt > 0); 985: require(id > 0); 1002: require(id > 0);
File: contracts/rubiconPools/BathHouse.sol 111: require(_reserveRatio > 0); 281: require(rr > 0);
There are 13 instances of this issue:
File: contracts/RubiconMarket.sol 990: uint256 old_top = 0;
File: contracts/rubiconPools/BathPair.sol 311: for (uint256 index = 0; index < array.length; index++) { 427: for (uint256 index = 0; index < quantity; index++) { 480: for (uint256 index = 0; index < quantity; index++) { 582: for (uint256 index = 0; index < ids.length; index++) {
File: contracts/rubiconPools/BathToken.sol 635: for (uint256 index = 0; index < bonusTokens.length; index++) {
File: contracts/RubiconRouter.sol 82: uint256 lastBid = 0; 83: uint256 lastAsk = 0; 85: for (uint256 index = 0; index < topNOrders; index++) { 168: uint256 currentAmount = 0; 169: for (uint256 i = 0; i < route.length - 1; i++) { 226: uint256 currentAmount = 0; 227: for (uint256 i = 0; i < route.length - 1; i++) {
internal functions not called by the contract should be removed to save deployment gasIf the functions are required by an interface, the contract should inherit from that interface and use the override keyword
There are 7 instances of this issue:
File: contracts/RubiconMarket.sol 61: function max(uint256 x, uint256 y) internal pure returns (uint256 z) { 65: function imin(int256 x, int256 y) internal pure returns (int256 z) { 69: function imax(int256 x, int256 y) internal pure returns (int256 z) { 76: function wmul(uint256 x, uint256 y) internal pure returns (uint256 z) { 441: function getFeeBPS() internal view returns (uint256) { 959: function _buys(uint256 id, uint256 amount) internal returns (bool) { 1113 function _offeru( 1114 uint256 pay_amt, //maker (ask) sell how much 1115 ERC20 pay_gem, //maker (ask) sell which token 1116 uint256 buy_amt, //maker (ask) buy how much 1117 ERC20 buy_gem //maker (ask) buy which token 1118: ) internal returns (uint256 id) {
++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)Saves 6 gas per loop
There are 8 instances of this issue:
File: contracts/rubiconPools/BathPair.sol 311: for (uint256 index = 0; index < array.length; index++) { 427: for (uint256 index = 0; index < quantity; index++) { 480: for (uint256 index = 0; index < quantity; index++) { 582: for (uint256 index = 0; index < ids.length; index++) {
File: contracts/rubiconPools/BathToken.sol 635: for (uint256 index = 0; index < bonusTokens.length; index++) {
File: contracts/RubiconRouter.sol 85: for (uint256 index = 0; index < topNOrders; index++) { 169: for (uint256 i = 0; i < route.length - 1; i++) { 227: for (uint256 i = 0; i < route.length - 1; i++) {
require() statements that use && saves gasSee this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper
There are 8 instances of this issue:
File: contracts/RubiconMarket.sol 715 require( 716 !isActive(id) && 717 _rank[id].delb != 0 && 718 _rank[id].delb < block.number - 10 719: ); 1177 require( 1178 _rank[id].delb == 0 && //assert id is in the sorted list 1179 isOfferSorted(id) 1180: );
File: contracts/rubiconPools/BathPair.sol 120 require( 121 IBathHouse(_bathHouse).getMarket() != 122 address(0x0000000000000000000000000000000000000000) && 123 IBathHouse(_bathHouse).initialized(), 124 "BathHouse not initialized" 125: ); 346 require( 347 bathAssetAddress != address(0) && bathQuoteAddress != address(0), 348 "tokenToBathToken error" 349: ); 419 require( 420 askNumerators.length == askDenominators.length && 421 askDenominators.length == bidNumerators.length && 422 bidNumerators.length == bidDenominators.length, 423 "not all order lengths match" 424: ); 471 require( 472 askNumerators.length == askDenominators.length && 473 askDenominators.length == bidNumerators.length && 474 bidNumerators.length == bidDenominators.length && 475 ids.length == askNumerators.length, 476 "not all input lengths match" 477: ); 506 require( 507 _bathAssetAddress != address(0) && _bathQuoteAddress != address(0), 508 "tokenToBathToken error" 509: );
File: contracts/rubiconPools/BathToken.sol 740 require( 741 recoveredAddress != address(0) && recoveredAddress == owner, 742 "bathToken: INVALID_SIGNATURE" 743: );
uints/ints smaller than 32 bytes (256 bits) incurs overheadWhen using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.
https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.html Use a larger size then downcast where needed
There are 37 instances of this issue:
File: contracts/RubiconMarket.sol 129: uint128 pay_amt, 130: uint128 buy_amt, 131: uint64 timestamp 140: uint128 pay_amt, 141: uint128 buy_amt, 142: uint64 timestamp 152: uint128 take_amt, 153: uint128 give_amt, 154: uint64 timestamp 163: uint128 pay_amt, 164: uint128 buy_amt, 165: uint64 timestamp 177: uint64 timestamp 205: uint64 timestamp; 385: uint128 pay_amt, 386: uint128 buy_amt 431: function take(bytes32 id, uint128 maxTakeAmount) external virtual { 475: function getTime() public view returns (uint64) { 584: uint128 pay_amt, 585: uint128 buy_amt 590: function take(bytes32 id, uint128 maxTakeAmount) public override {
File: contracts/rubiconPools/BathHouse.sol 52: uint8 public bpsToStrategists; 235: int128 _shapeCoefNum 359: function getBPSToStrats() public view returns (uint8) {
File: contracts/rubiconPools/BathPair.sol 35: int128 internal deprecatedStorageVarKept420Proxy; 115: function initialize(uint256 _maxOrderSizeBPS, int128 _shapeCoefNum) 512: uint16 stratReward = IBathHouse(_bathHouse).getBPSToStrats(); 542: uint24 _poolFee 545: uint16 stratRewardBPS = IBathHouse(bathHouse).getBPSToStrats();
File: contracts/rubiconPools/BathToken.sol 27: uint8 public decimals; 718: uint8 v,
File: contracts/peripheral_contracts/BathBuddy.sol 32: uint64 public start; 33: uint64 public duration; 40: uint64 startTimestamp, 41: uint64 durationSeconds 133: function vestedAmount(address token, uint64 timestamp) 149: function _vestingSchedule(uint256 totalAllocation, uint64 timestamp)
private rather than public for constants, saves gasIf needed, the value can be read from the verified contract source code. Savings are due to the compiler not having to create non-payable getter functions for deployment calldata, and not adding another entry to the method ID table
There is 1 instance of this issue:
File: contracts/rubiconPools/BathToken.sol #1 70 bytes32 public constant PERMIT_TYPEHASH = 71: 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9;
if (<x> == true) => if (<x>), if (<x> == false) => if (!<x>)
There are 4 instances of this issue:
File: contracts/rubiconPools/BathHouse.sol #1 242: IBathPair(_bathPairAddress).initialized() != true,
File: contracts/rubiconPools/BathHouse.sol #2 372: approvedStrategists[wouldBeStrategist] == true ||
File: contracts/rubiconPools/BathPair.sol #3 149 IBathHouse(bathHouse).isApprovedStrategist(targetStrategist) == 150: true,
File: contracts/rubiconPools/BathToken.sol #4 228: IBathHouse(bathHouse).isApprovedPair(msg.sender) == true,
require()/revert() checks should be refactored to a modifier or functionSaves deployment costs
There are 12 instances of this issue:
File: contracts/RubiconMarket.sol 216: require(isActive(id)); 595: require(cancel(uint256(id))); 591: require(buy(uint256(id), maxTakeAmount)); 460: require(!isClosed()); 645: require(!locked, "Reentrancy attempt"); 1119: require(_dust[address(pay_gem)] <= pay_amt); 1209: require(!isOfferSorted(id)); //make sure offer id is not in sorted offers list 879: require(offerId != 0); 1002: require(id > 0);
File: contracts/rubiconPools/BathToken.sol 547 require( 548 owner == msg.sender, 549 "This implementation does not support non-sender owners from withdrawing user shares" 550: );
File: contracts/RubiconRouter.sol 247: require(currentAmount >= buy_amt_min, "didnt clear buy_amt_min"); 481: require(target == ERC20(wethAddress), "target pool not weth pool");
<x> / 2 is the same as <x> >> 1. The DIV opcode costs 5 gas, whereas SHR only costs 3 gas
There are 4 instances of this issue:
File: contracts/RubiconMarket.sol #1 77: z = add(mul(x, y), WAD / 2) / WAD;
File: contracts/RubiconMarket.sol #2 81: z = add(mul(x, y), RAY / 2) / RAY;
File: contracts/RubiconMarket.sol #3 85: z = add(mul(x, WAD), y / 2) / y;
File: contracts/RubiconMarket.sol #4 89: z = add(mul(x, RAY), y / 2) / y;
require() or revert() statements that check input arguments should be at the top of the functionChecks that involve constants should come before checks that involve state variables
There are 4 instances of this issue:
File: contracts/RubiconMarket.sol #1 283: require(uint128(quantity) == quantity, "quantity is not an int");
File: contracts/RubiconMarket.sol #2 646: require(_dust[address(pay_gem)] <= pay_amt);
File: contracts/rubiconPools/BathHouse.sol #3 110: require(_reserveRatio <= 100);
File: contracts/rubiconPools/BathHouse.sol #4 111: require(_reserveRatio > 0);
The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation. If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}})
There are 3 instances of this issue:
File: contracts/peripheral_contracts/BathBuddy.sol #1 69: receive() external payable {}
File: contracts/RubiconRouter.sol #2 37: receive() external payable {}
File: contracts/RubiconRouter.sol #3 39: fallback() external payable {}
block.timestamp and block.number are added to event information by default so adding them manually wastes gas
There are 8 instances of this issue:
File: contracts/RubiconMarket.sol 131: uint64 timestamp 142: uint64 timestamp 154: uint64 timestamp 165: uint64 timestamp 177: uint64 timestamp
File: contracts/rubiconPools/BathHouse.sol 69: uint256 timestamp,
File: contracts/rubiconPools/BathPair.sol 88: uint256 timestamp, 108: uint256 timestamp
payableIf a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are
CALLVALUE(2),DUP1(3),ISZERO(3),PUSH2(3),JUMPI(10),PUSH1(3),DUP1(3),REVERT(0),JUMPDEST(1),POP(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost
There are 35 instances of this issue:
File: contracts/rubiconPools/BathHouse.sol 208 function createBathToken(ERC20 underlyingERC20, address _feeAdmin) 209 external 210 onlyAdmin 211: returns (address newBathTokenAddress) 217 function adminWriteBathToken(ERC20 overwriteERC20, address newBathToken) 218 external 219: onlyAdmin 232 function initBathPair( 233 address _bathPairAddress, 234 uint256 _maxOrderSizeBPS, 235 int128 _shapeCoefNum 236: ) external onlyAdmin returns (address newPair) { 253: function setBathHouseAdmin(address newAdmin) external onlyAdmin { 259: function setNewBathTokenImplementation(address newImplementation) external onlyAdmin { 264: function approveStrategist(address strategist) public onlyAdmin { 269: function setPermissionedStrategists(bool _new) external onlyAdmin { 274: function setCancelTimeDelay(uint256 value) external onlyAdmin { 279: function setReserveRatio(uint256 rr) external onlyAdmin { 286 function setBathTokenMarket(address bathToken, address newMarket) 287 external 288: onlyAdmin 294 function setBonusToken(address bathToken, address newBonusToken) 295 external 296: onlyAdmin 302 function setBathTokenBathHouse(address bathToken, address newAdmin) 303 external 304: onlyAdmin 310 function setBathTokenFeeBPS(address bathToken, uint256 newBPS) 311 external 312: onlyAdmin 319 function bathTokenApproveSetMarket(address targetBathToken) 320 external 321: onlyAdmin 327 function setBathTokenFeeTo(address bathToken, address feeTo) 328 external 329: onlyAdmin 335: function setMarket(address newMarket) external onlyAdmin {
File: contracts/rubiconPools/BathPair.sol 324 function placeMarketMakingTrades( 325 address[2] memory tokenPair, // ASSET, Then Quote 326 uint256 askNumerator, // Quote / Asset 327 uint256 askDenominator, // Asset / Quote 328 uint256 bidNumerator, // size in ASSET 329 uint256 bidDenominator // size in QUOTES 330: ) public onlyApprovedStrategist(msg.sender) returns (uint256 id) { 412 function batchMarketMakingTrades( 413 address[2] memory tokenPair, // ASSET, Then Quote 414 uint256[] memory askNumerators, // Quote / Asset 415 uint256[] memory askDenominators, // Asset / Quote 416 uint256[] memory bidNumerators, // size in ASSET 417 uint256[] memory bidDenominators // size in QUOTES 418: ) external onlyApprovedStrategist(msg.sender) { 440 function requote( 441 uint256 id, 442 address[2] memory tokenPair, // ASSET, Then Quote 443 uint256 askNumerator, // Quote / Asset 444 uint256 askDenominator, // Asset / Quote 445 uint256 bidNumerator, // size in ASSET 446 uint256 bidDenominator // size in QUOTES 447: ) public onlyApprovedStrategist(msg.sender) { 463 function batchRequoteOffers( 464 uint256[] memory ids, 465 address[2] memory tokenPair, // ASSET, Then Quote 466 uint256[] memory askNumerators, // Quote / Asset 467 uint256[] memory askDenominators, // Asset / Quote 468 uint256[] memory bidNumerators, // size in ASSET 469 uint256[] memory bidDenominators // size in QUOTES 470: ) external onlyApprovedStrategist(msg.sender) { 493 function rebalancePair( 494 uint256 assetRebalAmt, //amount of ASSET in the quote buffer 495 uint256 quoteRebalAmt, //amount of QUOTE in the asset buffer 496 address _underlyingAsset, 497 address _underlyingQuote 498: ) external onlyApprovedStrategist(msg.sender) { 535 function tailOff( 536 address targetPool, 537 address tokenToHandle, 538 address targetToken, 539 address _stratUtil, // delegatecall target 540 uint256 amount, //fill amount to handle 541 uint256 hurdle, //must clear this on tail off 542 uint24 _poolFee 543: ) external onlyApprovedStrategist(msg.sender) { 566 function scrubStrategistTrade(uint256 id) 567 public 568: onlyApprovedStrategist(msg.sender) 578 function scrubStrategistTrades(uint256[] memory ids) 579 external 580: onlyApprovedStrategist(msg.sender) 591 function strategistBootyClaim(address asset, address quote) 592 external 593: onlyApprovedStrategist(msg.sender)
File: contracts/rubiconPools/BathToken.sol 245: function setMarket(address newRubiconMarket) external onlyBathHouse { 250: function setBathHouse(address newBathHouse) external onlyBathHouse { 255: function approveMarket() external onlyBathHouse { 260: function setFeeBPS(uint256 _feeBPS) external onlyBathHouse { 265: function setFeeTo(address _feeTo) external onlyBathHouse { 270: function setBonusToken(address newBonusERC20) external onlyBathHouse { 279: function cancel(uint256 id, uint256 amt) external onlyPair { 294: function removeFilledTradeAmount(uint256 amt) external onlyPair { 306 function placeOffer( 307 uint256 pay_amt, 308 ERC20 pay_gem, 309 uint256 buy_amt, 310 ERC20 buy_gem 311: ) external onlyPair returns (uint256) { 346 function rebalance( 347 address destination, 348 address filledAssetToRebalance, /* sister or fill asset */ 349 uint256 stratProportion, 350 uint256 rebalAmt 351: ) external onlyPair {
#0 - liveactionllama
2022-05-27T16:44:23Z
Warden created this issue as a placeholder, because their submission was too large for the contest form. They then emailed their md file to our team on 05/27/2022. I've updated this issue with their md file content.