Skip to content

Commit

Permalink
changes after PR review
Browse files Browse the repository at this point in the history
  • Loading branch information
Daniel Graczer authored and Daniel Graczer committed Sep 22, 2022
1 parent 0f058ef commit 7fab122
Show file tree
Hide file tree
Showing 18 changed files with 45 additions and 46 deletions.
4 changes: 2 additions & 2 deletions Readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ The most important thing is to bump the major version number at each breaking ch
### Git tagging

Currently we have a very simple git workflow. New strategies are released from the main branch
and tagged by part of the 'name' of the strategy. For example if a new version of the cash strategy is released, and the current 'name' property in the source code of that strategy has the value of **block42.cash_strategy.cash_strategy_v1.0.2**, then the following git commands has to be executed.
and tagged by part of the 'name' of the strategy. For example if a new version of the cash strategy is released, and the current 'name' property in the source code of that strategy has the value of **brokkr.cash_strategy.cash_strategy_v1.0.2**, then the following git commands has to be executed.

```bash
git checkout main
git pull
# the tag name will be the the string following the last dot of the name, in the example the name was
# block42.cash_strategy.cash_strategy_v1.0.2
# brokkr.cash_strategy.cash_strategy_v1.0.2
git tag -a cash_strategy_v1.0.2
# the command above will open a text editor, please write down how this version is different from the previuos one
git push origin cash_strategy_v1.0.2
Expand Down
8 changes: 4 additions & 4 deletions contracts/common/bases/PortfolioBaseUpgradeable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ abstract contract PortfolioBaseUpgradeable is
Math.SHORT_FIXED_DECIMAL_FACTOR /
100;
if (embeddedAmount == 0) continue;
depositToken.safeApprove(
depositToken.approve(
address(investableDescs[i].investable),
embeddedAmount
);
Expand Down Expand Up @@ -294,7 +294,7 @@ abstract contract PortfolioBaseUpgradeable is
.getInvestmentTokenBalanceOf(address(this)) * amount) /
investmentTokenSupply;
if (embeddedTokenAmountToBurn == 0) continue;
embeddedInvestable.getInvestmentToken().safeApprove(
embeddedInvestable.getInvestmentToken().approve(
address(embeddedInvestable),
embeddedTokenAmountToBurn
);
Expand Down Expand Up @@ -366,7 +366,7 @@ abstract contract PortfolioBaseUpgradeable is
address(this)
) * targetInvestableEquities[i]) /
currentInvestableEquities[i];
embeddedInvestable.getInvestmentToken().safeApprove(
embeddedInvestable.getInvestmentToken().approve(
address(embeddedInvestable),
withdrawAmount
);
Expand All @@ -392,7 +392,7 @@ abstract contract PortfolioBaseUpgradeable is
);

if (depositAmount != 0) {
depositToken.safeApprove(
depositToken.approve(
address(embeddedInvestable),
depositAmount
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ contract PercentageAllocation is
{
// solhint-disable-next-line const-name-snakecase
string public constant name =
"block42.percentage_allocation_portfolio.percentage_allocation_portfolio_v1.0.1";
"brokkr.percentage_allocation_portfolio.percentage_allocation_portfolio_v1.0.1";
// solhint-disable-next-line const-name-snakecase
string public constant humanReadableName =
"Percentage allocation portfolio";
Expand Down
2 changes: 1 addition & 1 deletion contracts/strategies/cash/Cash.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract Cash is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
// solhint-disable-next-line const-name-snakecase
string public constant name = "block42.cash_strategy.cash_strategy_v1.0.1";
string public constant name = "brokkr.cash_strategy.cash_strategy_v1.0.1";
// solhint-disable-next-line const-name-snakecase
string public constant humanReadableName = "Cash strategy";
// solhint-disable-next-line const-name-snakecase
Expand Down
4 changes: 2 additions & 2 deletions contracts/strategies/stargate/Stargate.sol
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ contract Stargate is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
uint256 lpBalanceBefore = strategyStorage.lpToken.balanceOf(
address(this)
);
strategyStorage.poolDepositToken.safeApprove(
strategyStorage.poolDepositToken.approve(
address(strategyStorage.router),
amount
);
Expand All @@ -105,7 +105,7 @@ contract Stargate is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {

uint256 lpBalanceIncrement = lpBalanceAfter - lpBalanceBefore;

strategyStorage.lpToken.safeApprove(
strategyStorage.lpToken.approve(
address(strategyStorage.lpStaking),
lpBalanceIncrement
);
Expand Down
2 changes: 1 addition & 1 deletion contracts/strategies/template/Template.sol
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import "@openzeppelin/contracts-upgradeable/token/ERC20/utils/SafeERC20Upgradeab
contract Template is StrategyOwnablePausableBaseUpgradeable {
// solhint-disable-next-line const-name-snakecase
string public constant name =
"block42.template_strategy.<insert git label here>";
"brokkr.template_strategy.<insert git label here>";
// solhint-disable-next-line const-name-snakecase
string public constant humanReadableName = "Template strategy";
// solhint-disable-next-line const-name-snakecase
Expand Down
2 changes: 1 addition & 1 deletion contracts/tests/MockPortfolio.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import "../common/bases/PortfolioOwnableBaseUpgradeable.sol";
contract MockPortfolio is PortfolioOwnableBaseUpgradeable {
// solhint-disable-next-line const-name-snakecase
string public constant name =
"block42.mock_portfolio.<insert git label here>";
"brokkr.mock_portfolio.<insert git label here>";
// solhint-disable-next-line const-name-snakecase
string public constant humanReadableName = "Mock portfolio";
// solhint-disable-next-line const-name-snakecase
Expand Down
2 changes: 1 addition & 1 deletion contracts/tests/MockStrategy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ contract MockStrategy is
{
// solhint-disable-next-line const-name-snakecase
string public constant name =
"block42.mock_strategy.<insert git label here>";
"brokkr.mock_strategy.<insert git label here>";
// solhint-disable-next-line const-name-snakecase
string public constant humanReadableName = "Mock strategy";
// solhint-disable-next-line const-name-snakecase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ contract PercentageAllocationV2 is
{
// solhint-disable-next-line const-name-snakecase
string public constant name =
"block42.percentage_allocation_portfolio.percentage_allocation_portfolio_v2.0.0";
"brokkr.percentage_allocation_portfolio.percentage_allocation_portfolio_v2.0.0";
// solhint-disable-next-line const-name-snakecase
string public constant humanReadableName =
"Percentage allocation portfolio";
Expand Down
2 changes: 1 addition & 1 deletion contracts/tests/strategies/cash/CashV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";

contract CashV2 is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
// solhint-disable-next-line const-name-snakecase
string public constant name = "block42.cash_strategy.cash_strategy_initial";
string public constant name = "brokkr.cash_strategy.cash_strategy_initial";
// solhint-disable-next-line const-name-snakecase
string public constant humanReadableName = "Cash strategy";
// solhint-disable-next-line const-name-snakecase
Expand Down
4 changes: 2 additions & 2 deletions contracts/tests/strategies/stargate/StargateV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ contract StargateV2 is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {
uint256 lpBalanceBefore = strategyStorage.lpToken.balanceOf(
address(this)
);
strategyStorage.poolDepositToken.safeApprove(
strategyStorage.poolDepositToken.approve(
address(strategyStorage.router),
amount
);
Expand All @@ -105,7 +105,7 @@ contract StargateV2 is UUPSUpgradeable, StrategyOwnablePausableBaseUpgradeable {

uint256 lpBalanceIncrement = lpBalanceAfter - lpBalanceBefore;

strategyStorage.lpToken.safeApprove(
strategyStorage.lpToken.approve(
address(strategyStorage.lpStaking),
lpBalanceIncrement
);
Expand Down
5 changes: 2 additions & 3 deletions contracts/tests/strategies/traderjoe/TraderJoeV2.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ contract TraderJoeV2 is
{
using SafeERC20Upgradeable for IInvestmentToken;
using SafeERC20Upgradeable for IERC20Upgradeable;
using SafeERC20Upgradeable for ITraderJoePair;

error InvalidTraderJoeLpToken();

Expand Down Expand Up @@ -98,11 +97,11 @@ contract TraderJoeV2 is
);
uint256 depositTokenDesired = amount - swapAmount;

strategyStorage.pairDepositToken.safeApprove(
strategyStorage.pairDepositToken.approve(
address(strategyStorage.router),
pairDepositTokenDesired
);
depositToken.safeApprove(
depositToken.approve(
address(strategyStorage.router),
depositTokenDesired
);
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"version": "1.0.0",
"description": "template for project setup",
"main": "index.js",
"author": "block42",
"author": "brokkr",
"license": "MIT",
"devDependencies": {
"@aave/core-v3": "^1.13.1",
Expand Down
24 changes: 12 additions & 12 deletions test/portfolio/UnifiedRebalance.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@ import { getErrorRange } from "../shared/utils"

export function testRebalance() {
describe("Rebalance", async function () {
it("should succeed when the owner user rebalances - [100%, 0%, 0%] -> [33%, 33%, 34%]", async function () {
it("should succeed when the owner user rebalances - for example [100%, 0%, 0%] -> [33%, 33%, 34%]", async function () {
const investableLength = (await this.portfolio.getInvestables()).length

if (investableLength <= 1) {
return
}

// Set target allocations 100% to the first investable and 0% to the others. [100%, 0%, 0%]
// Set target allocations 100% to the first investable and 0% to the others. for example [100%, 0%, 0%]
let allocations: number[] = [100000]
for (let i = 1; i < investableLength; i++) {
allocations.push(0)
Expand All @@ -40,7 +40,7 @@ export function testRebalance() {
getErrorRange(ethers.utils.parseUnits("10000", 6))
)

// Set target allocations approximately equally. [33%, 33%, 34%]
// Set target allocations approximately equally. for example [33%, 33%, 34%]
allocations = []
const equalAllocation = Math.floor(100 / investableLength - 1)
for (let i = 0; i < investableLength - 1; i++) {
Expand Down Expand Up @@ -93,14 +93,14 @@ export function testRebalance() {
}
})

it("should succeed when the owner user rebalances - [50%, 50%, 0%] -> [33%, 33%, 34%]", async function () {
it("should succeed when the owner user rebalances - for example [50%, 50%, 0%] -> [33%, 33%, 34%]", async function () {
const investableLength = (await this.portfolio.getInvestables()).length

if (investableLength <= 1) {
return
}

// Set target allocations 50% to the first and second investable and 0% to the others. [50%, 50%, 0%]
// Set target allocations 50% to the first and second investable and 0% to the others. for example [50%, 50%, 0%]
let allocations: number[] = [50000, 50000]
for (let i = 2; i < investableLength; i++) {
allocations.push(0)
Expand All @@ -126,7 +126,7 @@ export function testRebalance() {
getErrorRange(ethers.utils.parseUnits("10000", 6))
)

// Set target allocations approximately equally. [33%, 33%, 34%]
// Set target allocations approximately equally. for example [33%, 33%, 34%]
allocations = []
const equalAllocation = Math.floor(100 / investableLength - 1)
for (let i = 0; i < investableLength - 1; i++) {
Expand Down Expand Up @@ -179,7 +179,7 @@ export function testRebalance() {
}
})

it("should succeed when the owner user rebalances and another user deposits into investable directly - [50%, 50%, 0%] -> [33%, 33%, 34%]", async function () {
it("should succeed when the owner user rebalances and another user deposits into investable directly - for example [50%, 50%, 0%] -> [33%, 33%, 34%]", async function () {
const investableLength = (await this.portfolio.getInvestables()).length

if (investableLength <= 1) {
Expand All @@ -194,7 +194,7 @@ export function testRebalance() {
await expect(investable.connect(this.user2).deposit(ethers.utils.parseUnits("3000", 6), this.user2.address, []))
.not.to.be.reverted

// Set target allocations 50% to the first and second investable and 0% to the others. [50%, 50%, 0%]
// Set target allocations 50% to the first and second investable and 0% to the others. for example [50%, 50%, 0%]
let allocations: number[] = [50000, 50000]
for (let i = 2; i < investableLength; i++) {
allocations.push(0)
Expand All @@ -220,7 +220,7 @@ export function testRebalance() {
getErrorRange(ethers.utils.parseUnits("10000", 6))
)

// Set target allocations approximately equally. [33%, 33%, 34%]
// Set target allocations approximately equally. for example [33%, 33%, 34%]
allocations = []
const equalAllocation = Math.floor(100 / investableLength - 1)
for (let i = 0; i < investableLength - 1; i++) {
Expand Down Expand Up @@ -279,7 +279,7 @@ export function testRebalance() {
}
})

it("should succeed when the owner user rebalances and another user withdraws from investable directly - [50%, 50%, 0%] ->[33%, 33%, 34%]", async function () {
it("should succeed when the owner user rebalances and another user withdraws from investable directly - for example [50%, 50%, 0%] -> [33%, 33%, 34%]", async function () {
const investableLength = (await this.portfolio.getInvestables()).length

if (investableLength <= 1) {
Expand All @@ -294,7 +294,7 @@ export function testRebalance() {
await expect(investable.connect(this.user2).deposit(ethers.utils.parseUnits("3000", 6), this.user2.address, []))
.not.to.be.reverted

// Set target allocations 50% to the first and second investable and 0% to the others. [50%, 50%, 0%]
// Set target allocations 50% to the first and second investable and 0% to the others. for example [50%, 50%, 0%]
let allocations: number[] = [50000, 50000]
for (let i = 2; i < investableLength; i++) {
allocations.push(0)
Expand Down Expand Up @@ -326,7 +326,7 @@ export function testRebalance() {
await expect(investable.connect(this.user2).withdraw(ethers.utils.parseUnits("1500", 6), this.user2.address, []))
.not.to.be.reverted

// Set target allocations approximately equally. [33%, 33%, 34%]
// Set target allocations approximately equally. for example [33%, 33%, 34%]
allocations = []
const equalAllocation = Math.floor(100 / investableLength - 1)
for (let i = 0; i < investableLength - 1; i++) {
Expand Down
2 changes: 1 addition & 1 deletion test/portfolio/UnifiedWithdraw.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@ export function testWithdraw() {
it("should succeed when a single user withdraws InvestmentToken that he/she has - full withdrawal", async function () {
await this.usdc.connect(this.user0).approve(this.portfolio.address, ethers.utils.parseUnits("3000", 6))
await this.portfolio.connect(this.user0).deposit(ethers.utils.parseUnits("3000", 6), this.user0.address, [])
await this.investmentToken.connect(this.user0).approve(this.portfolio.address, ethers.utils.parseUnits("3000", 6))

const availableTokenBalance = await this.investmentToken.balanceOf(this.user0.address)
await this.investmentToken.connect(this.user0).approve(this.portfolio.address, availableTokenBalance)
await expect(this.portfolio.connect(this.user0).withdraw(availableTokenBalance, this.user0.address, []))
.to.emit(this.portfolio, "Withdrawal")
.withArgs(this.user0.address, this.user0.address, availableTokenBalance)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ function testPercentageAllocationPortfolioUpgradeable() {

// IInvestable.
expect(await this.portfolio.name()).to.equal(
"block42.percentage_allocation_portfolio.percentage_allocation_portfolio_v2.0.0"
"brokkr.percentage_allocation_portfolio.percentage_allocation_portfolio_v2.0.0"
)
expect(await this.portfolio.humanReadableName()).to.equal("Percentage allocation portfolio")
expect(await this.portfolio.version()).to.equal("2.0.0")
Expand Down
20 changes: 10 additions & 10 deletions test/strategy/UnifiedWithdraw.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,13 @@ export function testWithdraw() {
await this.usdc.connect(this.user1).approve(this.strategy.address, ethers.utils.parseUnits("30", 6))
await this.strategy.connect(this.user1).deposit(ethers.utils.parseUnits("30", 6), this.user1.address, [])

// The first user partial withdraws.
// The first user partially withdraws.
await this.investmentToken.connect(this.user0).approve(this.strategy.address, ethers.utils.parseUnits("15", 6))
await expect(this.strategy.connect(this.user0).withdraw(ethers.utils.parseUnits("15", 6), this.user0.address, []))
.to.emit(this.strategy, "Withdrawal")
.withArgs(this.user0.address, this.user0.address, ethers.utils.parseUnits("15", 6))

// The second user partial withdraws.
// The second user partially withdraws.
const investmentTokenBalance = await this.investmentToken.balanceOf(this.user1.address)
const investmentTokenBalanceHalf = Math.floor(investmentTokenBalance / 2)
await this.investmentToken.connect(this.user1).approve(this.strategy.address, investmentTokenBalanceHalf)
Expand Down Expand Up @@ -276,22 +276,22 @@ export function testWithdraw() {
await this.strategy.connect(this.user0).deposit(ethers.utils.parseUnits("30", 6), this.user0.address, [])

// The first user fully withdraws.
let investmentTokenBalance = await this.investmentToken.balanceOf(this.user0.address)
await this.investmentToken.connect(this.user0).approve(this.strategy.address, investmentTokenBalance)
await expect(this.strategy.connect(this.user0).withdraw(investmentTokenBalance, this.user0.address, []))
let availableTokenBalance = await this.investmentToken.balanceOf(this.user0.address)
await this.investmentToken.connect(this.user0).approve(this.strategy.address, availableTokenBalance)
await expect(this.strategy.connect(this.user0).withdraw(availableTokenBalance, this.user0.address, []))
.to.emit(this.strategy, "Withdrawal")
.withArgs(this.user0.address, this.user0.address, investmentTokenBalance)
.withArgs(this.user0.address, this.user0.address, availableTokenBalance)

// The second user deposits.
await this.usdc.connect(this.user1).approve(this.strategy.address, ethers.utils.parseUnits("30", 6))
await this.strategy.connect(this.user1).deposit(ethers.utils.parseUnits("30", 6), this.user1.address, [])

// The second user fully withdraws.
investmentTokenBalance = await this.investmentToken.balanceOf(this.user1.address)
await this.investmentToken.connect(this.user1).approve(this.strategy.address, investmentTokenBalance)
await expect(this.strategy.connect(this.user1).withdraw(investmentTokenBalance, this.user1.address, []))
availableTokenBalance = await this.investmentToken.balanceOf(this.user1.address)
await this.investmentToken.connect(this.user1).approve(this.strategy.address, availableTokenBalance)
await expect(this.strategy.connect(this.user1).withdraw(availableTokenBalance, this.user1.address, []))
.to.emit(this.strategy, "Withdrawal")
.withArgs(this.user1.address, this.user1.address, investmentTokenBalance)
.withArgs(this.user1.address, this.user1.address, availableTokenBalance)

expect(await this.usdc.balanceOf(this.user0.address)).to.be.approximately(
ethers.utils.parseUnits("100", 6),
Expand Down
2 changes: 1 addition & 1 deletion test/strategy/cash/Cash.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ function testCashUpgradeable() {
expect(equityValuationBefore.eq(equityValuationAfter)).to.equal(true)

// IInvestable.
expect(await this.strategy.name()).to.equal("block42.cash_strategy.cash_strategy_initial")
expect(await this.strategy.name()).to.equal("brokkr.cash_strategy.cash_strategy_initial")
expect(await this.strategy.humanReadableName()).to.equal("Cash strategy")
expect(await this.strategy.version()).to.equal("2.0.0")
})
Expand Down

0 comments on commit 7fab122

Please sign in to comment.