Skip to content

Commit

Permalink
fix: remove Holding struct
Browse files Browse the repository at this point in the history
  • Loading branch information
@maximebrugel
maximebrugel committed Jan 3, 2022
1 parent 9ee6af1 commit c924109
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 69 deletions.
2 changes: 1 addition & 1 deletion contracts/NestedBuybacker.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ contract NestedBuybacker is Ownable {
feeSplitter = FeeSplitter(_feeSplitter);
nstReserve = _nstReserve;
}

/// @notice Update the nested reserve address
/// @param _nstReserve New reserve contract address
function setNestedReserve(address _nstReserve) external onlyOwner {
Expand Down
28 changes: 15 additions & 13 deletions contracts/NestedFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,10 +212,10 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, Ownable, MixinOperato
uint256 buyTokenInitialBalance = _buyToken.balanceOf(address(this));

for (uint256 i = 0; i < tokensLength; i++) {
NestedRecords.Holding memory holding = nestedRecords.getAssetHolding(_nftId, tokens[i]);
reserve.withdraw(IERC20(tokens[i]), holding.amount);
uint256 amount = nestedRecords.getAssetHolding(_nftId, tokens[i]);
reserve.withdraw(IERC20(tokens[i]), amount);

_safeSubmitOrder(tokens[i], address(_buyToken), holding.amount, _nftId, _orders[i]);
_safeSubmitOrder(tokens[i], address(_buyToken), amount, _nftId, _orders[i]);
nestedRecords.freeHolding(_nftId, tokens[i]);
}

Expand Down Expand Up @@ -249,12 +249,9 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, Ownable, MixinOperato

address token = nestedRecords.getAssetTokens(_nftId)[_tokenIndex];

NestedRecords.Holding memory holding = nestedRecords.getAssetHolding(
_nftId,
token
);
reserve.withdraw(IERC20(token), holding.amount);
_safeTransferWithFees(IERC20(token), holding.amount, _msgSender(), _nftId);
uint256 amount = nestedRecords.getAssetHolding(_nftId, token);
reserve.withdraw(IERC20(token), amount);
_safeTransferWithFees(IERC20(token), amount, _msgSender(), _nftId);

nestedRecords.deleteAsset(_nftId, _tokenIndex);

Expand Down Expand Up @@ -451,8 +448,10 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, Ownable, MixinOperato
bool _fromReserve
) private returns (IERC20 tokenUsed) {
if (_fromReserve) {
NestedRecords.Holding memory holding = nestedRecords.getAssetHolding(_nftId, address(_inputToken));
require(holding.amount >= _inputTokenAmount, "NF: INSUFFICIENT_AMOUNT_IN");
require(
nestedRecords.getAssetHolding(_nftId, address(_inputToken)) >= _inputTokenAmount,
"NF: INSUFFICIENT_AMOUNT_IN"
);

// Get input from reserve
reserve.withdraw(IERC20(_inputToken), _inputTokenAmount);
Expand Down Expand Up @@ -509,8 +508,11 @@ contract NestedFactory is INestedFactory, ReentrancyGuard, Ownable, MixinOperato
address _inputToken,
uint256 _amount
) private {
NestedRecords.Holding memory holding = nestedRecords.getAssetHolding(_nftId, _inputToken);
nestedRecords.updateHoldingAmount(_nftId, _inputToken, holding.amount - _amount);
nestedRecords.updateHoldingAmount(
_nftId,
_inputToken,
nestedRecords.getAssetHolding(_nftId, _inputToken) - _amount
);
}

/// @dev Transfer a token amount from the factory to the recipient.
Expand Down
29 changes: 11 additions & 18 deletions contracts/NestedRecords.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,11 @@ contract NestedRecords is OwnableFactoryHandler {
/// @dev Emitted when the reserve is updated for a specific portfolio
/// @param nftId The NFT ID
/// @param newReserve The new reserve address
event reserveUpdated(uint256 nftId, address newReserve);

/// @dev Info about assets stored in reserves
struct Holding {
uint256 amount;
bool isActive;
}
event reserveUpdated(uint256 nftId, address newReserve);

/// @dev Store user asset informations
struct NftRecord {
mapping(address => Holding) holdings;
mapping(address => uint256) holdings;
address[] tokens;
address reserve;
uint256 lockTimestamp;
Expand Down Expand Up @@ -64,7 +58,7 @@ contract NestedRecords is OwnableFactoryHandler {
tokenIndex++;
}
} else {
records[_nftId].holdings[_token].amount = _amount;
records[_nftId].holdings[_token] = _amount;
}
}

Expand Down Expand Up @@ -92,10 +86,10 @@ contract NestedRecords is OwnableFactoryHandler {
uint256 _amount,
address _reserve
) external onlyFactory {
Holding memory holding = records[_nftId].holdings[_token];
if (holding.isActive) {
uint256 amount = records[_nftId].holdings[_token];
if (amount != 0) {
require(records[_nftId].reserve == _reserve, "NRC: RESERVE_MISMATCH");
updateHoldingAmount(_nftId, _token, holding.amount + _amount);
updateHoldingAmount(_nftId, _token, amount + _amount);
return;
}
require(records[_nftId].tokens.length < maxHoldingsCount, "NRC: TOO_MANY_TOKENS");
Expand All @@ -104,15 +98,15 @@ contract NestedRecords is OwnableFactoryHandler {
"NRC: INVALID_RESERVE"
);

records[_nftId].holdings[_token] = Holding({ amount: _amount, isActive: true });
records[_nftId].holdings[_token] = _amount;
records[_nftId].tokens.push(_token);
records[_nftId].reserve = _reserve;
}

/// @notice Get holding object for this NFT ID
/// @notice Get holding amount for this NFT ID
/// @param _nftId The id of the NFT
/// @param _token The address of the token
function getAssetHolding(uint256 _nftId, address _token) external view returns (Holding memory) {
function getAssetHolding(uint256 _nftId, address _token) external view returns (uint256) {
return records[_nftId].holdings[_token];
}

Expand Down Expand Up @@ -176,12 +170,11 @@ contract NestedRecords is OwnableFactoryHandler {
function deleteAsset(uint256 _nftId, uint256 _tokenIndex) public onlyFactory {
address[] storage tokens = records[_nftId].tokens;
address token = tokens[_tokenIndex];
Holding memory holding = records[_nftId].holdings[token];

require(holding.isActive, "NRC: HOLDING_INACTIVE");
require(records[_nftId].holdings[token] != 0, "NRC: HOLDING_INACTIVE");

delete records[_nftId].holdings[token];
tokens[_tokenIndex] = tokens[tokens.length - 1];
tokens.pop();
}
}
}
4 changes: 2 additions & 2 deletions test/unit/FlatOperator.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ describe("FlatOperator", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(uniBought);
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(uniBought);
});

it("remove token from portfolio when destroy()", async () => {
Expand Down
70 changes: 35 additions & 35 deletions test/unit/NestedFactory.unit.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ describe("NestedFactory", () => {
await expect(
context.nestedFactory
.connect(context.user1)
.create(0, context.mockDAI.address, totalToSpend, orders, {value: 1}),
.create(0, context.mockDAI.address, totalToSpend, orders, { value: 1 }),
).to.be.revertedWith("NF: UNSUPPORTED_ETH_TRANSFER");
});

Expand Down Expand Up @@ -375,10 +375,10 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(uniBought);
const holdingsKNC = await context.nestedRecords.getAssetHolding(1, context.mockKNC.address);
expect(holdingsKNC.amount).to.be.equal(kncBought);
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(uniBought);
const holdingsKNCAmount = await context.nestedRecords.getAssetHolding(1, context.mockKNC.address);
expect(holdingsKNCAmount).to.be.equal(kncBought);
});

it("Creates NFT from DAI with KNI and UNI inside (ZeroExOperator) with more than needed", async () => {
Expand Down Expand Up @@ -730,10 +730,10 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(uniBought.add(baseUniBought));
const holdingsKNC = await context.nestedRecords.getAssetHolding(1, context.mockKNC.address);
expect(holdingsKNC.amount).to.be.equal(kncBought.add(baseKncBought));
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(uniBought.add(baseUniBought));
const holdingsKNCAmount = await context.nestedRecords.getAssetHolding(1, context.mockKNC.address);
expect(holdingsKNCAmount).to.be.equal(kncBought.add(baseKncBought));
});

it("increase KNI and UNI amount from DAI (ZeroExOperator) with more than needed", async () => {
Expand Down Expand Up @@ -823,12 +823,12 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(baseUniBought);
const holdingsKNC = await context.nestedRecords.getAssetHolding(1, context.mockKNC.address);
expect(holdingsKNC.amount).to.be.equal(baseKncBought);
const holdingsDAI = await context.nestedRecords.getAssetHolding(1, context.mockDAI.address);
expect(holdingsDAI.amount).to.be.equal(daiBought);
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(baseUniBought);
const holdingsKNCAmount = await context.nestedRecords.getAssetHolding(1, context.mockKNC.address);
expect(holdingsKNCAmount).to.be.equal(baseKncBought);
const holdingsDAIAmount = await context.nestedRecords.getAssetHolding(1, context.mockDAI.address);
expect(holdingsDAIAmount).to.be.equal(daiBought);
});
});

Expand Down Expand Up @@ -1016,8 +1016,8 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(uniBought.add(baseUniBought));
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(uniBought.add(baseUniBought));
});

it("increase UNI amount from KNC in portfolio (ZeroExOperator) with more than needed", async () => {
Expand Down Expand Up @@ -1091,12 +1091,12 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(baseUniBought.sub(totalToSpend));
const holdingsKNC = await context.nestedRecords.getAssetHolding(1, context.mockKNC.address);
expect(holdingsKNC.amount).to.be.equal(baseKncBought);
const holdingsUSDC = await context.nestedRecords.getAssetHolding(1, context.mockUSDC.address);
expect(holdingsUSDC.amount).to.be.equal(usdcBought);
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(baseUniBought.sub(totalToSpend));
const holdingsKNCAmount = await context.nestedRecords.getAssetHolding(1, context.mockKNC.address);
expect(holdingsKNCAmount).to.be.equal(baseKncBought);
const holdingsUSDCAmount = await context.nestedRecords.getAssetHolding(1, context.mockUSDC.address);
expect(holdingsUSDCAmount).to.be.equal(usdcBought);
});
});

Expand Down Expand Up @@ -1280,12 +1280,12 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(baseUniBought.sub(uniSold));
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(baseUniBought.sub(uniSold));

// Must have the right amount in the holdings
const holdingsUSDC = await context.nestedRecords.getAssetHolding(1, context.mockUSDC.address);
expect(holdingsUSDC.amount).to.be.equal(usdcBought.sub(expectedUsdcFees));
const holdingsUSDCAmount = await context.nestedRecords.getAssetHolding(1, context.mockUSDC.address);
expect(holdingsUSDCAmount).to.be.equal(usdcBought.sub(expectedUsdcFees));
});

it("swap KNC and UNI for USDC (ZeroExOperator) with more than needed", async () => {
Expand Down Expand Up @@ -1363,11 +1363,11 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(baseUniBought.sub(uniSold));
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(baseUniBought.sub(uniSold));

const holdingsUSDC = await context.nestedRecords.getAssetHolding(1, context.mockUSDC.address);
expect(holdingsUSDC.amount).to.be.equal(usdcBoughtOrder.sub(orderExpectedFee));
const holdingsUSDCAmount = await context.nestedRecords.getAssetHolding(1, context.mockUSDC.address);
expect(holdingsUSDCAmount).to.be.equal(usdcBoughtOrder.sub(orderExpectedFee));
});
});

Expand Down Expand Up @@ -1550,8 +1550,8 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(baseUniBought.sub(uniSold));
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(baseUniBought.sub(uniSold));

expect(await context.mockUSDC.balanceOf(context.user1.address)).to.be.equal(
context.baseAmount.add(usdcBought.sub(expectedUsdcFees)),
Expand Down Expand Up @@ -1630,8 +1630,8 @@ describe("NestedFactory", () => {
);

// Must have the right amount in the holdings
const holdingsUNI = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNI.amount).to.be.equal(baseUniBought.sub(uniSold));
const holdingsUNIAmount = await context.nestedRecords.getAssetHolding(1, context.mockUNI.address);
expect(holdingsUNIAmount).to.be.equal(baseUniBought.sub(uniSold));
});
});

Expand Down

0 comments on commit c924109

Please sign in to comment.