From f0ae1dfeb19d20947aae3f08203f723aed1a2d0e Mon Sep 17 00:00:00 2001 From: pizzaman1337 Date: Mon, 29 Jul 2024 12:31:05 +0200 Subject: [PATCH 1/8] H-01 LibChainlinkOracle::getTokenPrice will always return instantaneous prices --- protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol b/protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol index 68230b9832..8fcbeefd74 100644 --- a/protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol +++ b/protocol/contracts/libraries/Oracle/LibChainlinkOracle.sol @@ -42,7 +42,7 @@ library LibChainlinkOracle { uint256 lookback ) internal view returns (uint256 price) { return - lookback > 0 + lookback == 0 ? getPrice(priceAggregatorAddress, maxTimeout) : getTwap(priceAggregatorAddress, maxTimeout, lookback); } From a70bdfca2eb8dbf753049d67d2b2f93c0cf9e38d Mon Sep 17 00:00:00 2001 From: pizzaman1337 Date: Mon, 29 Jul 2024 18:18:52 +0200 Subject: [PATCH 2/8] H-02 LibUsdOracle returns the wrong price for Uniswap Oracle --- .../libraries/Oracle/LibUniswapOracle.sol | 40 +++++++++++++++---- .../libraries/Oracle/LibUsdOracle.sol | 14 +++---- .../libraries/Oracle/LibWstethEthOracle.sol | 5 ++- protocol/test/foundry/silo/Oracle.t.sol | 10 ++++- 4 files changed, 51 insertions(+), 18 deletions(-) diff --git a/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol b/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol index 44dcc74862..6f5bce0374 100644 --- a/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol +++ b/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol @@ -9,6 +9,10 @@ import {C} from "contracts/C.sol"; import {OracleLibrary} from "@uniswap/v3-periphery/contracts/libraries/OracleLibrary.sol"; import "@uniswap/v3-core/contracts/interfaces/IUniswapV3Pool.sol"; +interface IERC20Decimals { + function decimals() external view returns (uint8); +} + /** * @title Uniswap Oracle Library * @notice Contains functionalty to read prices from Uniswap V3 pools. @@ -19,23 +23,43 @@ import "@uniswap/v3-core/contracts/interfaces/IUniswapV3Pool.sol"; library LibUniswapOracle { // All instantaneous queries of Uniswap Oracles should use a 15 minute lookback. uint32 internal constant FIFTEEN_MINUTES = 900; + uint256 constant PRECISION = 1e6; /** - * @dev Uses `pool`'s Uniswap V3 Oracle to get the TWAP price of `token1` in `token2` over the - * last `lookback` seconds. - * Return value has 6 decimal precision. - * Returns 0 if {IUniswapV3Pool.observe} reverts. + * @notice Given a tick and a token amount, calculates the amount of token received in exchange + * @param baseTokenAmount Amount of baseToken to be converted. + * @param baseToken Address of the ERC20 token contract used as the baseAmount denomination. + * @param quoteToken Address of the ERC20 token contract used as the quoteAmount denomination. + * @return price Amount of quoteToken. Value has 6 decimal precision. */ function getTwap( uint32 lookback, address pool, - address token1, - address token2, - uint128 oneToken + address baseToken, + address quoteToken, + uint128 baseTokenAmount ) internal view returns (uint256 price) { (bool success, int24 tick) = consult(pool, lookback); if (!success) return 0; - price = OracleLibrary.getQuoteAtTick(tick, oneToken, token1, token2); + + price = OracleLibrary.getQuoteAtTick(tick, baseTokenAmount, baseToken, quoteToken); + + uint256 baseTokenDecimals = IERC20Decimals(baseToken).decimals(); + uint256 quoteTokenDecimals = IERC20Decimals(quoteToken).decimals(); + int256 factor = int256(baseTokenDecimals) - int256(quoteTokenDecimals); + + // decimals are the same. i.e. DAI/WETH + if (factor == 0) return (price * PRECISION) / (10 ** baseTokenDecimals); + + // scale decimals + if (factor > 0) { + price = price * (10 ** uint256(factor)); + } else { + price = price / (10 ** uint256(-factor)); + } + + // set 1e6 precision + price = (price * PRECISION) / (10 ** baseTokenDecimals); } /** diff --git a/protocol/contracts/libraries/Oracle/LibUsdOracle.sol b/protocol/contracts/libraries/Oracle/LibUsdOracle.sol index dd936b42c6..6d5408661d 100644 --- a/protocol/contracts/libraries/Oracle/LibUsdOracle.sol +++ b/protocol/contracts/libraries/Oracle/LibUsdOracle.sol @@ -111,9 +111,9 @@ library LibUsdOracle { if (chainlinkOraclePriceAddress == address(0)) { // use the chainlink registry chainlinkOraclePriceAddress = ChainlinkPriceFeedRegistry(chainlinkRegistry).getFeed( - token, - 0x0000000000000000000000000000000000000348 - ); // 0x0348 is the address for USD + token, + 0x0000000000000000000000000000000000000348 + ); // 0x0348 is the address for USD } return @@ -134,8 +134,8 @@ library LibUsdOracle { tokenPrice = LibUniswapOracle.getTwap( lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES : uint32(lookback), oracleImpl.target, - chainlinkToken, token, + chainlinkToken, uint128(10) ** uint128(IERC20Decimals(token).decimals()) ); // USDC/USD @@ -146,9 +146,9 @@ library LibUsdOracle { if (chainlinkOraclePriceAddress == address(0)) { // use the chainlink registry chainlinkOraclePriceAddress = ChainlinkPriceFeedRegistry(chainlinkRegistry).getFeed( - chainlinkToken, - 0x0000000000000000000000000000000000000348 - ); // 0x0348 is the address for USD + chainlinkToken, + 0x0000000000000000000000000000000000000348 + ); // 0x0348 is the address for USD } uint256 chainlinkTokenPrice = LibChainlinkOracle.getTokenPrice( diff --git a/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol b/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol index 00041bbcd9..b7de31cd50 100644 --- a/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol +++ b/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol @@ -82,11 +82,14 @@ library LibWstethEthOracle { uint256 uniswapPrice = LibUniswapOracle.getTwap( lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES : uint32(lookback), C.WSTETH_ETH_UNIV3_01_POOL, - C.WSTETH, C.WETH, + C.WSTETH, ONE ); + // uniswap price is a 1e6, need to convert to 1e18, multiply by 1e12 + uniswapPrice = uniswapPrice * PRECISION_DENOMINATOR; + // Check if the uniswapPrice oracle fails. if (uniswapPrice == 0) return 0; diff --git a/protocol/test/foundry/silo/Oracle.t.sol b/protocol/test/foundry/silo/Oracle.t.sol index 62022f511d..111948440a 100644 --- a/protocol/test/foundry/silo/Oracle.t.sol +++ b/protocol/test/foundry/silo/Oracle.t.sol @@ -16,7 +16,7 @@ contract OracleTest is TestHelper { function test_getUsdPrice() public { // encode type 0x01 uint256 price = OracleFacet(BEANSTALK).getUsdPrice(WBTC); - assertEq(price, 50000e6); + assertEq(price, 50000e6, "price using encode type 0x01"); // change encode type to 0x02: vm.prank(BEANSTALK); @@ -25,6 +25,12 @@ contract OracleTest is TestHelper { IMockFBeanstalk.Implementation(WBTC_USDC_03_POOL, bytes4(0), bytes1(0x02)) ); price = OracleFacet(BEANSTALK).getUsdPrice(WBTC); - assertApproxEqRel(price, 50000e6, 0.001e18); + // 1 USDC will get ~500 satoshis of BTC at $50k + // 1 USDC = 1e6 + // 1 wBTC = 1e8 + // $50,000/100000000*1000000 = 500 + // function returns uint256(1e24).div(tokenPrice); + // expected delta is 0.2004008016032064% + assertApproxEqRel(price, 1e24 / 500, 2.1e15, "price using encode type 0x02"); } } From 2ff0a8b1b6ee289d5f4373d34bd477fac2515aca Mon Sep 17 00:00:00 2001 From: pizzaman1337 Date: Mon, 29 Jul 2024 23:11:23 +0200 Subject: [PATCH 3/8] L-02 Lack of Validation for minFillAmount Less Than or Equal to podAmount Results in Unfillable Listings and Denial of Service --- .../market/MarketplaceFacet/Listing.sol | 4 + protocol/test/foundry/field/Marketplace.t.sol | 83 +++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 protocol/test/foundry/field/Marketplace.t.sol diff --git a/protocol/contracts/beanstalk/market/MarketplaceFacet/Listing.sol b/protocol/contracts/beanstalk/market/MarketplaceFacet/Listing.sol index e0ae8a1d89..c22b6ff346 100644 --- a/protocol/contracts/beanstalk/market/MarketplaceFacet/Listing.sol +++ b/protocol/contracts/beanstalk/market/MarketplaceFacet/Listing.sol @@ -66,6 +66,10 @@ contract Listing is PodTransfer { s.sys.fields[podListing.fieldId].harvestable <= podListing.maxHarvestableIndex, "Marketplace: Expired." ); + require( + podListing.minFillAmount <= podListing.podAmount, + "Marketplace: minFillAmount must be <= podAmount." + ); if (s.sys.podListings[podListing.fieldId][podListing.index] != bytes32(0)) LibMarket._cancelPodListing(podListing.lister, podListing.fieldId, podListing.index); diff --git a/protocol/test/foundry/field/Marketplace.t.sol b/protocol/test/foundry/field/Marketplace.t.sol new file mode 100644 index 0000000000..d5eb7f9bac --- /dev/null +++ b/protocol/test/foundry/field/Marketplace.t.sol @@ -0,0 +1,83 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.20; + +import "forge-std/Test.sol"; +import {TestHelper, IMockFBeanstalk} from "test/foundry/utils/TestHelper.sol"; +import {MockFieldFacet} from "contracts/mocks/mockFacets/MockFieldFacet.sol"; +import {MockSeasonFacet} from "contracts/mocks/mockFacets/MockSeasonFacet.sol"; +import {C} from "contracts/C.sol"; + +contract ListingTest is TestHelper { + // test accounts + address[] farmers; + + MockFieldFacet field = MockFieldFacet(BEANSTALK); + MockSeasonFacet season = MockSeasonFacet(BEANSTALK); + + function setUp() public { + initializeBeanstalkTestState(true, false); + + season.siloSunrise(0); + + // initalize farmers from farmers (farmer0 == diamond deployer) + farmers.push(users[1]); + farmers.push(users[2]); + + // max approve. + maxApproveBeanstalk(farmers); + + mintTokensToUsers(farmers, C.BEAN, MAX_DEPOSIT_BOUND); + + field.incrementTotalSoilE(1000e18); + + // mine 300 blocks + vm.roll(300); + + //set temp + bs.setYieldE(0); + + console.log("bs.activeField(): ", bs.activeField()); + + // sow 1000 + vm.prank(users[1]); + uint256 pods = bs.sow(1000e6, 0, 0); + console.log("Pods: ", pods); + vm.prank(users[2]); + bs.sow(1000e6, 0, 0); + } + + function testCreatePodListing_InvalidMinFillAmount() public { + IMockFBeanstalk.PodListing memory podListing = IMockFBeanstalk.PodListing({ + lister: users[1], + fieldId: bs.activeField(), + index: 0, + start: 0, + podAmount: 50, + pricePerPod: 100, + maxHarvestableIndex: 100, + minFillAmount: 60, // Invalid: greater than podAmount + mode: 0 + }); + + vm.expectRevert("Marketplace: minFillAmount must be <= podAmount."); + vm.prank(users[1]); + bs.createPodListing(podListing); + } + + function testCreatePodListing_ValidMinFillAmount() public { + // no revert + IMockFBeanstalk.PodListing memory podListing = IMockFBeanstalk.PodListing({ + lister: users[1], + fieldId: bs.activeField(), + index: 0, + start: 0, + podAmount: 50, + pricePerPod: 100, + maxHarvestableIndex: 100, + minFillAmount: 30, // Valid: less than or equal to podAmount + mode: 0 + }); + vm.prank(users[1]); + bs.createPodListing(podListing); + } +} From 1b2ba74281bc9bc93545601e3d5651627ede1d35 Mon Sep 17 00:00:00 2001 From: pizzaman1337 Date: Tue, 30 Jul 2024 23:16:27 +0200 Subject: [PATCH 4/8] Fix hardhat oracle tests --- protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol | 2 +- protocol/scripts/impersonate.js | 1 + protocol/test/foundry/utils/OracleDeployer.sol | 1 + protocol/utils/oracle.js | 6 +++--- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol b/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol index b7de31cd50..1a78c0acc9 100644 --- a/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol +++ b/protocol/contracts/libraries/Oracle/LibWstethEthOracle.sol @@ -82,8 +82,8 @@ library LibWstethEthOracle { uint256 uniswapPrice = LibUniswapOracle.getTwap( lookback == 0 ? LibUniswapOracle.FIFTEEN_MINUTES : uint32(lookback), C.WSTETH_ETH_UNIV3_01_POOL, - C.WETH, C.WSTETH, + C.WETH, ONE ); diff --git a/protocol/scripts/impersonate.js b/protocol/scripts/impersonate.js index db80f889c1..64a8d286e7 100644 --- a/protocol/scripts/impersonate.js +++ b/protocol/scripts/impersonate.js @@ -43,6 +43,7 @@ async function wsteth() { const wsteth = await ethers.getContractAt("MockWsteth", WSTETH); await wsteth.setSymbol("wstETH"); await wsteth.setStEthPerToken(to18("1")); + await wsteth.setDecimals(18); } /// Uniswap V2 Router /// diff --git a/protocol/test/foundry/utils/OracleDeployer.sol b/protocol/test/foundry/utils/OracleDeployer.sol index 17d2dde6f3..d83d477101 100644 --- a/protocol/test/foundry/utils/OracleDeployer.sol +++ b/protocol/test/foundry/utils/OracleDeployer.sol @@ -20,6 +20,7 @@ import {MockUniswapV3Factory} from "contracts/mocks/uniswap/MockUniswapV3Factory interface ChainlinkPriceFeedRegistry { function getFeed(address base, address quote) external view returns (address aggregator); } + contract OracleDeployer is Utils { ////////// CHAINLINK ////////// address constant USDC_USD_CHAINLINK_PRICE_AGGREGATOR = diff --git a/protocol/utils/oracle.js b/protocol/utils/oracle.js index 1ab5151a6b..432bbfd6b8 100644 --- a/protocol/utils/oracle.js +++ b/protocol/utils/oracle.js @@ -61,9 +61,9 @@ async function setStethEthChainlinkPrice(price, secondsAgo = 900) { ); } -async function setWstethUsdPrice(price) { - await setStethEthChainlinkPrice(price); - await setWstethEthUniswapPrice(price); +async function setWstethUsdPrice(price, wstethEthRatio = "1") { + await setStethEthChainlinkPrice(wstethEthRatio); + await setWstethEthUniswapPrice(wstethEthRatio); await setWstethStethRedemptionPrice("1"); await setEthUsdChainlinkPrice(price); } From 08b7789226739702688e02e893df138f3cca7ef9 Mon Sep 17 00:00:00 2001 From: pizzaman1337 Date: Mon, 5 Aug 2024 14:22:28 +0300 Subject: [PATCH 5/8] L-08 verify tests work as expected --- protocol/test/hardhat/SopReport.test.js | 181 ++++++++++++++++++++++++ 1 file changed, 181 insertions(+) create mode 100644 protocol/test/hardhat/SopReport.test.js diff --git a/protocol/test/hardhat/SopReport.test.js b/protocol/test/hardhat/SopReport.test.js new file mode 100644 index 0000000000..7dc52db4d0 --- /dev/null +++ b/protocol/test/hardhat/SopReport.test.js @@ -0,0 +1,181 @@ +const { expect } = require("chai"); +const { deploy } = require("../../scripts/deploy.js"); +const { EXTERNAL, INTERNAL, INTERNAL_EXTERNAL, INTERNAL_TOLERANT } = require("./utils/balances.js"); +const { + BEAN, + BEAN_ETH_WELL, + WETH, + MAX_UINT256, + ZERO_ADDRESS, + BEAN_WSTETH_WELL, + WSTETH +} = require("./utils/constants.js"); +const { to18, to6, advanceTime } = require("./utils/helpers.js"); +const { + deployMockWell, + whitelistWell, + deployMockWellWithMockPump +} = require("../../utils/well.js"); +const { takeSnapshot, revertToSnapshot } = require("./utils/snapshot.js"); +const { + setStethEthChainlinkPrice, + setWstethEthUniswapPrice, + setEthUsdChainlinkPrice +} = require("../../utils/oracle.js"); +const { getAllBeanstalkContracts } = require("../../utils/contracts.js"); + +let user, user2, owner; + +describe("Sop Test Cases", function () { + before(async function () { + [owner, user, user2] = await ethers.getSigners(); + + const contracts = await deploy((verbose = false), (mock = true), (reset = true)); + ownerAddress = contracts.account; + this.diamond = contracts.beanstalkDiamond; + + // `beanstalk` contains all functions that the regualar beanstalk has. + // `mockBeanstalk` has functions that are only available in the mockFacets. + [beanstalk, mockBeanstalk] = await getAllBeanstalkContracts(this.diamond.address); + + bean = await ethers.getContractAt("Bean", BEAN); + this.weth = await ethers.getContractAt("MockToken", WETH); + + mockBeanstalk.deployStemsUpgrade(); + + await mockBeanstalk.siloSunrise(0); + + await bean.connect(user).approve(beanstalk.address, "100000000000"); + await bean.connect(user2).approve(beanstalk.address, "100000000000"); + await bean.mint(user.address, to6("10000")); + await bean.mint(user2.address, to6("10000")); + + // init wells + [this.well, this.wellFunction, this.pump] = await deployMockWellWithMockPump(); + await deployMockWellWithMockPump(BEAN_WSTETH_WELL, WSTETH); + await this.well.connect(owner).approve(this.diamond.address, to18("100000000")); + await this.well.connect(user).approve(this.diamond.address, to18("100000000")); + + // set reserves at a 1000:1 ratio. + await this.pump.setCumulativeReserves(this.well.address, [to6("1000000"), to18("1000")]); + await this.well.mint(ownerAddress, to18("500")); + await this.well.mint(user.address, to18("500")); + await mockBeanstalk.siloSunrise(0); + await mockBeanstalk.captureWellE(this.well.address); + + await setEthUsdChainlinkPrice("1000"); + await setStethEthChainlinkPrice("1000"); + await setStethEthChainlinkPrice("1"); + await setWstethEthUniswapPrice("1"); + + await this.well.setReserves([to6("1000000"), to18("1100")]); + await this.pump.setInstantaneousReserves(this.well.address, [to6("1000000"), to18("1100")]); + + await mockBeanstalk.siloSunrise(0); // 3 -> 4 + await mockBeanstalk.siloSunrise(0); // 4 -> 5 + }); + + beforeEach(async function () { + snapshotId = await takeSnapshot(); + }); + + afterEach(async function () { + await revertToSnapshot(snapshotId); + }); + + describe("consecutive floods scenarios", async function () { + it("mowing between two consecutive floods, loses the rewards in the second flood", async function () { + console.log("current season: ", await beanstalk.season()); // 5 + + await beanstalk.connect(user).deposit(bean.address, to6("1000"), EXTERNAL); + + await mockBeanstalk.siloSunrise(0); // 5 => 6 + + await beanstalk.mow(user.address, bean.address); + + // rain starts in season 7 + await mockBeanstalk.rainSunrise(); // 6 => 7 + + const rain = await beanstalk.rain(); + let season = await beanstalk.time(); + + // there is a flood in season 8 + await mockBeanstalk.rainSunrise(); // 7 => 8 + + await beanstalk.mow(user.address, bean.address); + + const balanceOfPlentyFirst = await beanstalk + .connect(user) + .balanceOfPlenty(user.address, this.well.address); + console.log("balanceOfPlentyFirst: ", balanceOfPlentyFirst); + + // Changing the reserves to have SoP rewards in the next season, otherwise deltaB would be zero + // This is just to simulate a situation when after the flood in season 8, still the condition of + // P > 1 and pod rate < %5 are met, leading to another flood in the next season + await this.well.setReserves([to6("1000000"), to18("1100")]); + await this.pump.setInstantaneousReserves(this.well.address, [to6("1000000"), to18("1100")]); + + // there is a flood in season 9 + await mockBeanstalk.rainSunrise(); // 8 => 9 + + await beanstalk.mow(user.address, bean.address); + + const balanceOfPlenty = await beanstalk + .connect(user) + .balanceOfPlenty(user.address, this.well.address); + + console.log("user's balanceOfPlenty: ", balanceOfPlenty); + + await beanstalk.connect(user).claimPlenty(this.well.address, EXTERNAL); + console.log("balance: ", await this.weth.balanceOf(user.address)); + + expect(await this.weth.balanceOf(user.address)).to.be.equal(balanceOfPlenty); + }); + }); + + it("mowing after two consecutive floods, gain the rewards of both floods", async function () { + console.log("current season: ", await beanstalk.season()); // 5 + + await beanstalk.connect(user).deposit(bean.address, to6("1000"), EXTERNAL); + + await mockBeanstalk.siloSunrise(0); // 5 => 6 + + await beanstalk.mow(user.address, bean.address); + + // rain starts in season 7 + await mockBeanstalk.rainSunrise(); // 6 => 7 + + const rain = await beanstalk.rain(); + let season = await beanstalk.time(); + + // there is a flood in season 8 + await mockBeanstalk.rainSunrise(); // 7 => 8 + + const balanceOfPlentyFirst = await beanstalk + .connect(user) + .balanceOfPlenty(user.address, this.well.address); + console.log("balanceOfPlentyFirst: ", balanceOfPlentyFirst); + + // Changing the reserves to have SoP rewards in the next season, otherwise deltaB would be zero + // This is just to simulate a situation when after the flood in season 8, still the condition of + // P > 1 and pod rate < %5 are met, leading to another flood in the next season + await this.well.setReserves([to6("1000000"), to18("1100")]); + await this.pump.setInstantaneousReserves(this.well.address, [to6("1000000"), to18("1100")]); + + // there is a flood in season 9 + await mockBeanstalk.rainSunrise(); // 8 => 9 + + await beanstalk.mow(user.address, bean.address); + + const balanceOfPlenty = await beanstalk + .connect(user) + .balanceOfPlenty(user.address, this.well.address); + + console.log("user's balanceOfPlenty: ", balanceOfPlenty); + + await beanstalk.connect(user).claimPlenty(this.well.address, EXTERNAL); + console.log("balance: ", await this.weth.balanceOf(user.address)); + + expect(await this.weth.balanceOf(user.address)).to.be.equal(balanceOfPlenty); + }); +}); From 147d3e6cab4cf55692f2b4687326cde3379334d0 Mon Sep 17 00:00:00 2001 From: pizzaman1337 Date: Mon, 5 Aug 2024 21:17:26 +0300 Subject: [PATCH 6/8] Report #395 Fix unnecessary inversion in getTokenPriceFromExternal --- .../libraries/Oracle/LibUsdOracle.sol | 10 ++--- protocol/test/foundry/silo/Oracle.t.sol | 37 ++++++++++++++++++- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/protocol/contracts/libraries/Oracle/LibUsdOracle.sol b/protocol/contracts/libraries/Oracle/LibUsdOracle.sol index 6d5408661d..bfbe1c8e80 100644 --- a/protocol/contracts/libraries/Oracle/LibUsdOracle.sol +++ b/protocol/contracts/libraries/Oracle/LibUsdOracle.sol @@ -117,12 +117,10 @@ library LibUsdOracle { } return - uint256(1e24).div( - LibChainlinkOracle.getTokenPrice( - chainlinkOraclePriceAddress, - LibChainlinkOracle.FOUR_HOUR_TIMEOUT, - lookback - ) + LibChainlinkOracle.getTokenPrice( + chainlinkOraclePriceAddress, + LibChainlinkOracle.FOUR_HOUR_TIMEOUT, + lookback ); } else if (oracleImpl.encodeType == bytes1(0x02)) { // assumes a dollar stablecoin is passed in diff --git a/protocol/test/foundry/silo/Oracle.t.sol b/protocol/test/foundry/silo/Oracle.t.sol index 111948440a..38436feab7 100644 --- a/protocol/test/foundry/silo/Oracle.t.sol +++ b/protocol/test/foundry/silo/Oracle.t.sol @@ -13,10 +13,10 @@ contract OracleTest is TestHelper { initializeBeanstalkTestState(true, false); } - function test_getUsdPrice() public { + function testUniswapOracleImplementation() public { // encode type 0x01 uint256 price = OracleFacet(BEANSTALK).getUsdPrice(WBTC); - assertEq(price, 50000e6, "price using encode type 0x01"); + assertEq(price, 1e24 / 50000e6, "price using encode type 0x01"); // change encode type to 0x02: vm.prank(BEANSTALK); @@ -33,4 +33,37 @@ contract OracleTest is TestHelper { // expected delta is 0.2004008016032064% assertApproxEqRel(price, 1e24 / 500, 2.1e15, "price using encode type 0x02"); } + + function testGetTokenPrice() public view { + // token price is number of dollars per token, i.e. 50000 USD for 1 WBTC + uint256 tokenPriceEth = OracleFacet(BEANSTALK).getTokenPrice(C.WETH); // 1000e6 + assertEq(tokenPriceEth, 1000e6, "tokenPriceEth"); + + // number of tokens received per dollar + uint256 usdPriceEth = OracleFacet(BEANSTALK).getUsdPrice(C.WETH); // 1e15 which is 1e18 (1 eth in wei) / 1000 (weth price 1000), you get 1/1000th of 1 eth for $1 + assertEq(usdPriceEth, 1e18 / 1000, "usdPriceEth"); + + uint256 tokenPriceWBTC = OracleFacet(BEANSTALK).getTokenPrice(WBTC); // should be 50000e6 + assertEq(tokenPriceWBTC, 50000e6, "tokenPriceWBTC"); + // assertEq(tokenPrice, 50000e6, "price using encode type 0x01"); + + // number of tokens received per dollar + uint256 usdPriceWBTC = OracleFacet(BEANSTALK).getUsdPrice(WBTC); // $1 = 0.00002 wbtc, wbtc is 8 decimals. 1e24 / 50000e6 + assertEq(usdPriceWBTC, 1e24 / 50000e6, "usdPriceWBTC"); + } + + // test provided by T1MOH + function test_getUsdPrice_whenExternalToken_priceIsInvalid() public view { + // pre condition: encode type 0x01 + + // WETH price is 1000 + uint256 priceWETH = OracleFacet(BEANSTALK).getUsdPrice(C.WETH); + assertEq(priceWETH, 1e15); // 1e18/1e3 = 1e15 + + // WBTC price is 50000 + uint256 priceWBTC = OracleFacet(BEANSTALK).getUsdPrice(WBTC); + assertEq(priceWBTC, 2e13); // 1e24.div(50000e6) = 2e13 + } + + // TODO: fork tests to verify the on-chain values currently returned by oracles alignes with mocks? } From f4bc9152ec4ab677ed5eb2c8e5f7c541c00dc2da Mon Sep 17 00:00:00 2001 From: pizzaman1337 Date: Tue, 6 Aug 2024 12:10:46 +0300 Subject: [PATCH 7/8] M-05. quickSort function does not work as expected, compromising the calculation of Beans per Well to be minted during a flood --- .../contracts/libraries/Silo/LibFlood.sol | 33 ++++++--- protocol/test/foundry/sun/Flood.t.sol | 69 +++++++++++++++++-- 2 files changed, 87 insertions(+), 15 deletions(-) diff --git a/protocol/contracts/libraries/Silo/LibFlood.sol b/protocol/contracts/libraries/Silo/LibFlood.sol index 50c43ba728..f915782343 100644 --- a/protocol/contracts/libraries/Silo/LibFlood.sol +++ b/protocol/contracts/libraries/Silo/LibFlood.sol @@ -259,7 +259,6 @@ library LibFlood { quickSort(wellDeltaBs, 0, int(wellDeltaBs.length - 1)); } - // Reviewer note: This works, but there's got to be a way to make this more gas efficient function quickSort( WellDeltaB[] memory arr, int left, @@ -269,13 +268,29 @@ library LibFlood { // Choose the median of left, right, and middle as pivot (improves performance on random data) uint mid = uint(left) + (uint(right) - uint(left)) / 2; - WellDeltaB memory pivot = arr[uint(left)].deltaB > arr[uint(mid)].deltaB - ? ( - arr[uint(left)].deltaB < arr[uint(right)].deltaB - ? arr[uint(left)] - : arr[uint(right)] - ) - : (arr[uint(mid)].deltaB < arr[uint(right)].deltaB ? arr[uint(mid)] : arr[uint(right)]); + WellDeltaB memory pivot; + + if (arr[uint(left)].deltaB > arr[uint(mid)].deltaB) { + if (arr[uint(left)].deltaB < arr[uint(right)].deltaB) { + pivot = arr[uint(left)]; + } else { + if (arr[uint(right)].deltaB > arr[uint(mid)].deltaB) { + pivot = arr[uint(right)]; + } else { + pivot = arr[uint(mid)]; + } + } + } else { + if (arr[uint(mid)].deltaB < arr[uint(right)].deltaB) { + pivot = arr[uint(mid)]; + } else { + if (arr[uint(right)].deltaB > arr[uint(left)].deltaB) { + pivot = arr[uint(right)]; + } else { + pivot = arr[uint(left)]; + } + } + } int i = left; int j = right; @@ -290,7 +305,7 @@ library LibFlood { } if (left < j) { - return quickSort(arr, left, j); + arr = quickSort(arr, left, j); } if (i < right) { return quickSort(arr, i, right); diff --git a/protocol/test/foundry/sun/Flood.t.sol b/protocol/test/foundry/sun/Flood.t.sol index 7da35233af..ce4cd5af0d 100644 --- a/protocol/test/foundry/sun/Flood.t.sol +++ b/protocol/test/foundry/sun/Flood.t.sol @@ -739,13 +739,26 @@ contract FloodTest is TestHelper { wells[1] = LibFlood.WellDeltaB(address(1), 200); wells[2] = LibFlood.WellDeltaB(address(2), -300); wells[3] = LibFlood.WellDeltaB(address(3), 400); - wells[4] = LibFlood.WellDeltaB(address(4), -500); + wells[4] = LibFlood.WellDeltaB(address(4), 500); wells = LibFlood.quickSort(wells, 0, right); - assertEq(wells[0].deltaB, 400); - assertEq(wells[1].deltaB, 200); - assertEq(wells[2].deltaB, 100); - assertEq(wells[3].deltaB, -300); - assertEq(wells[4].deltaB, -500); + assertEq(wells[0].deltaB, 500); + assertEq(wells[1].deltaB, 400); + assertEq(wells[2].deltaB, 200); + assertEq(wells[3].deltaB, 100); + assertEq(wells[4].deltaB, -300); + + // these values are examples from the codehawks report + wells[0] = LibFlood.WellDeltaB(address(0), 39); + wells[1] = LibFlood.WellDeltaB(address(1), 6); + wells[2] = LibFlood.WellDeltaB(address(2), 27); + wells[3] = LibFlood.WellDeltaB(address(3), -14); + wells[4] = LibFlood.WellDeltaB(address(4), 15); + wells = LibFlood.quickSort(wells, 0, right); + assertEq(wells[0].deltaB, 39); + assertEq(wells[1].deltaB, 27); + assertEq(wells[2].deltaB, 15); + assertEq(wells[3].deltaB, 6); + assertEq(wells[4].deltaB, -14); wells = new LibFlood.WellDeltaB[](2); right = int(wells.length - 1); @@ -760,6 +773,50 @@ contract FloodTest is TestHelper { wells = LibFlood.quickSort(wells, 0, right); assertEq(wells[0].deltaB, 200); assertEq(wells[1].deltaB, 100); + + wells = new LibFlood.WellDeltaB[](20); + right = int(wells.length - 1); + wells[0] = LibFlood.WellDeltaB(address(0), -1); + wells[1] = LibFlood.WellDeltaB(address(1), 2); + wells[2] = LibFlood.WellDeltaB(address(2), -3); + wells[3] = LibFlood.WellDeltaB(address(3), 4); + wells[4] = LibFlood.WellDeltaB(address(4), -5); + wells[5] = LibFlood.WellDeltaB(address(5), 6); + wells[6] = LibFlood.WellDeltaB(address(6), -7); + wells[7] = LibFlood.WellDeltaB(address(7), 8); + wells[8] = LibFlood.WellDeltaB(address(8), -9); + wells[9] = LibFlood.WellDeltaB(address(9), 10); + wells[10] = LibFlood.WellDeltaB(address(10), -11); + wells[11] = LibFlood.WellDeltaB(address(11), 12); + wells[12] = LibFlood.WellDeltaB(address(12), -13); + wells[13] = LibFlood.WellDeltaB(address(13), 14); + wells[14] = LibFlood.WellDeltaB(address(14), -15); + wells[15] = LibFlood.WellDeltaB(address(15), 16); + wells[16] = LibFlood.WellDeltaB(address(16), -17); + wells[17] = LibFlood.WellDeltaB(address(17), 18); + wells[18] = LibFlood.WellDeltaB(address(18), -19); + wells[19] = LibFlood.WellDeltaB(address(19), 20); + wells = LibFlood.quickSort(wells, 0, right); + assertEq(wells[0].deltaB, 20); + assertEq(wells[1].deltaB, 18); + assertEq(wells[2].deltaB, 16); + assertEq(wells[3].deltaB, 14); + assertEq(wells[4].deltaB, 12); + assertEq(wells[5].deltaB, 10); + assertEq(wells[6].deltaB, 8); + assertEq(wells[7].deltaB, 6); + assertEq(wells[8].deltaB, 4); + assertEq(wells[9].deltaB, 2); + assertEq(wells[10].deltaB, -1); + assertEq(wells[11].deltaB, -3); + assertEq(wells[12].deltaB, -5); + assertEq(wells[13].deltaB, -7); + assertEq(wells[14].deltaB, -9); + assertEq(wells[15].deltaB, -11); + assertEq(wells[16].deltaB, -13); + assertEq(wells[17].deltaB, -15); + assertEq(wells[18].deltaB, -17); + assertEq(wells[19].deltaB, -19); } function test_notGerminated() public { From a4e89acbd7d7faf0f43913fab8584896d96f2f03 Mon Sep 17 00:00:00 2001 From: pizzaman1337 Date: Wed, 7 Aug 2024 09:52:26 +0300 Subject: [PATCH 8/8] Fix getTwap vars --- protocol/contracts/libraries/Oracle/LibUniswapOracle.sol | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol b/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol index e4e95a4ed1..ca6aa49e12 100644 --- a/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol +++ b/protocol/contracts/libraries/Oracle/LibUniswapOracle.sol @@ -42,7 +42,12 @@ library LibUniswapOracle { (bool success, int24 tick) = consult(pool, lookback); if (!success) return 0; - price = LibUniswapOracleLibrary.getQuoteAtTick(tick, oneToken, token1, token2); + price = LibUniswapOracleLibrary.getQuoteAtTick( + tick, + baseTokenAmount, + baseToken, + quoteToken + ); uint256 baseTokenDecimals = IERC20Decimals(baseToken).decimals(); uint256 quoteTokenDecimals = IERC20Decimals(quoteToken).decimals();