Skip to content
This repository has been archived by the owner on Oct 26, 2022. It is now read-only.

feat: lock on first deposit #102

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: lock on first deposit #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
{
"files.eol": "\n"
"files.eol": "\n",
"solidity.defaultCompiler": "remote"
}
10 changes: 5 additions & 5 deletions contracts/BentoBox.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,8 +185,6 @@ contract BentoBox is MasterContractManager, BoringBatchable {
IERC20 token = token_ == USE_ETHEREUM ? wethToken : token_;
Rebase memory total = totals[token];

// If a new token gets added, the tokenSupply call checks that this is a deployed contract. Needed for security.
require(total.elastic != 0 || token.totalSupply() > 0, "BentoBox: No tokens");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check protects against adding shares for a token that is yet to be deployed (predictable address with CREATE2)

if (share == 0) {
// value of the share may be lower than the amount due to rounding, that's ok
share = total.toBase(amount, false);
Expand All @@ -207,9 +205,13 @@ contract BentoBox is MasterContractManager, BoringBatchable {
"BentoBox: Skim too much"
);

balanceOf[token][to] = balanceOf[token][to].add(share);
total.base = total.base.add(share.to128());
if (total.elastic == 0) {
// Upon the first deposit permanently lock the MINIMUM_SHARE_BALANCE amount of tokens
share = share.sub(MINIMUM_SHARE_BALANCE);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YieldBox has a different solution for this that doesn't burn any user tokens, but adds a virtual balance instead. Would love your review of this new method :)

}
total.elastic = total.elastic.add(amount.to128());
balanceOf[token][to] = balanceOf[token][to].add(share);
totals[token] = total;

// Interactions
Expand Down Expand Up @@ -258,8 +260,6 @@ contract BentoBox is MasterContractManager, BoringBatchable {
balanceOf[token][from] = balanceOf[token][from].sub(share);
total.elastic = total.elastic.sub(amount.to128());
total.base = total.base.sub(share.to128());
// There have to be at least 1000 shares left to prevent reseting the share/amount ratio (unless it's fully emptied)
require(total.base >= MINIMUM_SHARE_BALANCE || total.base == 0, "BentoBox: cannot empty");
totals[token] = total;

// Interactions
Expand Down
40 changes: 21 additions & 19 deletions test/BentoBox.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ describe("BentoBox", function () {
.to.emit(this.a, "Transfer")
.withArgs(this.fred.address, this.bentoBox.address, getBigNumber(100))
.to.emit(this.bentoBox, "LogDeposit")
.withArgs(this.a.address, this.fred.address, this.fred.address, getBigNumber(100), getBigNumber(100))
.withArgs(this.a.address, this.fred.address, this.fred.address, getBigNumber(100), getBigNumber(100).sub(1000))

this.bentoBox.connect(this.fred).addProfit(this.a.address, getBigNumber(30))

Expand All @@ -64,7 +64,7 @@ describe("BentoBox", function () {
.to.emit(this.b, "Transfer")
.withArgs(this.fred.address, this.bentoBox.address, getBigNumber(200, 6))
.to.emit(this.bentoBox, "LogDeposit")
.withArgs(this.b.address, this.fred.address, this.fred.address, getBigNumber(200, 6), getBigNumber(200, 6))
.withArgs(this.b.address, this.fred.address, this.fred.address, getBigNumber(200, 6), getBigNumber(200, 6).sub(1000))

this.bentoBox.connect(this.fred).addProfit(this.b.address, getBigNumber(200, 6))

Expand Down Expand Up @@ -249,19 +249,21 @@ describe("BentoBox", function () {
.to.emit(this.weth9, "Deposit")
.withArgs(this.bentoBox.address, "1000")
.to.emit(this.bentoBox, "LogDeposit")
.withArgs(this.weth9.address, this.bob.address, this.bob.address, "1000", "1000")
.withArgs(this.weth9.address, this.bob.address, this.bob.address, "1000", "0")

expect(await this.weth9.balanceOf(this.bentoBox.address), "BentoBox should hold WETH").to.be.equal(1000)
expect(await this.bentoBox.balanceOf(this.weth9.address, this.bob.address), "bob shouldn't have weth").to.be.equal(0)
await this.bentoBox.connect(this.bob).deposit(ADDRESS_ZERO, this.bob.address, this.bob.address, 1000, 0, { value: 1000 })
expect(await this.bentoBox.balanceOf(this.weth9.address, this.bob.address), "bob should have weth").to.be.equal(1000)
})

it("Reverts if TotalSupply of token is Zero or if token isn't a token", async function () {
it("Reverts if token isn't a token", async function () {
// await expect(
// this.bentoBox.connect(this.bob).deposit(ADDRESS_ZERO, this.bob.address, this.bob.address, 1, 0, { value: 1 })
// ).to.be.revertedWith("BentoBox: No tokens")
await expect(
this.bentoBox.connect(this.bob).deposit(ADDRESS_ZERO, this.bob.address, this.bob.address, 1, 0, { value: 1 })
).to.be.revertedWith("BentoBox: No tokens")
await expect(
this.bentoBox.connect(this.bob).deposit(this.bentoBox.address, this.bob.address, this.bob.address, 1, 0, { value: 1 })
).to.be.revertedWith("Transaction reverted: function selector was not recognized and there's no fallback function")
this.bentoBox.connect(this.bob).deposit(this.bentoBox.address, this.bob.address, this.bob.address, 1000, 0)
).to.be.revertedWith("VM Exception while processing transaction: revert BoringERC20: TransferFrom failed")
})

it("Mutates balanceOf and totalSupply for two deposits correctly", async function () {
Expand Down Expand Up @@ -327,15 +329,15 @@ describe("BentoBox", function () {

it("should not allow grieving attack with deposit of Share", async function () {
await this.c.approve(this.bentoBox.address, 1000000000000)
await this.bentoBox.deposit(this.c.address, this.alice.address, this.alice.address, 0, 1)
await this.bentoBox.deposit(this.c.address, this.alice.address, this.c.address, 0, 1000)
await this.bentoBox.addProfit(this.c.address, 1)
let amount = 2
for (let i = 0; i < 20; i++) {
await this.bentoBox.deposit(this.c.address, this.alice.address, this.alice.address, amount - 1, 0)
amount += amount - 1
}
const ratio =
(await this.bentoBox.totals(this.c.address)).elastic / (await this.bentoBox.balanceOf(this.c.address, this.alice.address))
(await this.bentoBox.totals(this.c.address)).elastic.sub(1000) / (await this.bentoBox.balanceOf(this.c.address, this.alice.address))
expect(ratio).to.be.lessThan(5)
})
})
Expand Down Expand Up @@ -374,10 +376,10 @@ describe("BentoBox", function () {
.to.emit(this.a, "Transfer")
.withArgs(this.alice.address, this.bento.address, "1000")
.to.emit(this.bento, "LogDeposit")
.withArgs(this.a.address, this.alice.address, this.alice.address, "1000", "1000")
.withArgs(this.a.address, this.alice.address, this.alice.address, "1000", "0")

await expect(this.bento.withdraw(this.a.address, this.alice.address, this.alice.address, 0, 2)).to.be.revertedWith(
"BentoBox: cannot empty"
await expect(this.bento.withdraw(this.a.address, this.alice.address, this.alice.address, 0, 1)).to.be.revertedWith(
"VM Exception while processing transaction: revert BoringMath: Underflow"
)
})

Expand Down Expand Up @@ -447,7 +449,7 @@ describe("BentoBox", function () {
from: this.bob.address,
})

expect(await this.bentoBox.balanceOf(this.weth9.address, this.bob.address), "token should be withdrawn").to.be.equal(100000)
expect(await this.bentoBox.balanceOf(this.weth9.address, this.bob.address), "token should be withdrawn").to.be.equal(100000 - 1000)
})

it("Reverts if ETH transfer fails", async function () {
Expand Down Expand Up @@ -714,20 +716,20 @@ describe("BentoBox", function () {
describe("Skim ETH", function () {
it("Skims ether to from address", async function () {
await this.weth9.connect(this.alice).deposit({
value: 1000,
value: 2000,
})

await this.bentoBox.batch([], true, {
value: 1000,
value: 2000,
})

await this.bentoBox.deposit(ADDRESS_ZERO, this.bentoBox.address, this.alice.address, 1000, 0)
await this.bentoBox.deposit(ADDRESS_ZERO, this.bentoBox.address, this.alice.address, 2000, 0)

amount = await this.bentoBox.balanceOf(this.weth9.address, this.alice.address)

expect(amount, "alice should have weth").to.equal(1000)

expect(await this.weth9.balanceOf(this.bentoBox.address), "BentoBox should hold WETH").to.equal(1000)
expect(await this.weth9.balanceOf(this.bentoBox.address), "BentoBox should hold WETH").to.equal(2000)
})
})

Expand Down
2 changes: 1 addition & 1 deletion test/HelloWorld.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe("HelloWorld", function () {

it("should allow deposit", async function () {
await this.helloWorld.deposit(APPROVAL_AMOUNT)
assert.equal((await this.helloWorld.balance()).toString(), APPROVAL_AMOUNT.toString())
assert.equal((await this.helloWorld.balance()).toString(), (APPROVAL_AMOUNT - 1000).toString())
})

it("should allow withdraw", async function () {
Expand Down
2 changes: 1 addition & 1 deletion test/harness/harvest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe("DummyStrategyMock", function () {
it("check balances of BentoBox", async function () {
assert.equal(
(await this.bentoBox.balanceOf(this.tokenA.address, this.alice.address)).toString(),
getBigNumber(DEPOSIT_AMOUNT, await this.tokenA.decimals()).toString(),
getBigNumber(DEPOSIT_AMOUNT, await this.tokenA.decimals()).sub(1000).toString(),
"should match deposit amount"
)

Expand Down