Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hotfix: Duplicate Currency in Bitmap #92

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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

Large diffs are not rendered by default.

Large diffs are not rendered by default.

6 changes: 4 additions & 2 deletions build/deployments/map.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
{
"1": {
"AccountAction": [
"0xf9FDb607d8b843cdF65898df4E08d1479b69002a",
"0x5f11E94E0A69ac8490f45eB27a6478dCDDB0227E"
],
"BatchAction": [
Expand All @@ -21,8 +22,8 @@
"0xc2763a6465F5C86769A1Af826dB270A6367b03B7"
],
"GovernorAlpha": [
"0x95DF7E34403beCd532f2BE160cacda56F0BD6bA3",
"0x086b4ecD75c494dD36641195E89c25373E06d7cB"
"0x086b4ecD75c494dD36641195E89c25373E06d7cB",
"0x95DF7E34403beCd532f2BE160cacda56F0BD6bA3"
],
"InitializeMarketsAction": [
"0x280deCD520da16e5571A6f2Fb803A57e0c16f423"
Expand All @@ -43,6 +44,7 @@
"0xbe401d7e76bb71bf7fa5a4aed7F3b650C6E0bd25"
],
"Router": [
"0x2Cc280279b8572fCDC100e2d01CA594a00031CB0",
"0x19152dDa25a96d0Ca244f0d7f3F13a966F392b23",
"0x123fCA954EA894305b684F56A0d043169a5aA7E4"
],
Expand Down
7 changes: 3 additions & 4 deletions contracts/external/actions/AccountAction.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,16 @@ contract AccountAction is ActionGuards {
using SafeInt256 for int256;

/// @notice Enables a bitmap currency for msg.sender, account cannot have any assets when this call
/// occurs.
/// @param currencyId the currency to enable the bitmap for. If set to zero then will attempt to disable
/// the bitmap portfolio for the account
/// occurs. Will revert if the account already has a bitmap currency set.
/// @param currencyId the currency to enable the bitmap for.
/// @dev emit:AccountSettled emit:AccountContextUpdate
/// @dev auth:msg.sender
function enableBitmapCurrency(uint16 currencyId) external {
require(msg.sender != address(this)); // dev: no internal call to enableBitmapCurrency
require(currencyId <= maxCurrencyId); // dev: invalid currency id
address account = msg.sender;
(AccountContext memory accountContext, /* didSettle */) = _settleAccountIfRequired(account);
accountContext.enableBitmapForAccount(account, currencyId, block.timestamp);
accountContext.enableBitmapForAccount(currencyId, block.timestamp);
accountContext.setAccountContext(account);
}

Expand Down
33 changes: 15 additions & 18 deletions contracts/internal/AccountContextHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,31 +30,28 @@ library AccountContextHandler {
return accountContext.bitmapCurrencyId != 0;
}

/// @notice Sets the account context of a given account
/// @notice Enables a bitmap type portfolio for an account. A bitmap type portfolio allows
/// an account to hold more fCash than a normal portfolio, except only in a single currency.
/// Once enabled, it cannot be disabled or changed. An account can only enable a bitmap if
/// it has no assets or debt so that we ensure no assets are left stranded.
/// @param accountContext refers to the account where the bitmap will be enabled
/// @param currencyId the id of the currency to enable
/// @param blockTime the current block time to set the next settle time
function enableBitmapForAccount(
AccountContext memory accountContext,
address account,
uint16 currencyId,
uint256 blockTime
) internal view {
// Allow setting the currency id to zero to turn off bitmap
require(currencyId <= Constants.MAX_CURRENCIES, "AC: invalid currency id");

if (isBitmapEnabled(accountContext)) {
// Account cannot change their bitmap if they have assets set
bytes32 ifCashBitmap =
BitmapAssetsHandler.getAssetsBitmap(account, accountContext.bitmapCurrencyId);
require(ifCashBitmap == 0, "AC: cannot have assets");
} else {
require(accountContext.assetArrayLength == 0, "AC: cannot have assets");
// Account context also cannot have negative cash debts
require(accountContext.hasDebt == 0x00, "AC: cannot have debt");
require(!isBitmapEnabled(accountContext), "Cannot change bitmap");
require(0 < currencyId && currencyId <= Constants.MAX_CURRENCIES, "Invalid currency id");

// Ensure that the active currency is set to false in the array so that there is no double
// counting during FreeCollateral
setActiveCurrency(accountContext, currencyId, false, Constants.ACTIVE_IN_BALANCES);
}
// Account cannot have assets or debts
require(accountContext.assetArrayLength == 0, "Cannot have assets");
require(accountContext.hasDebt == 0x00, "Cannot have debt");

// Ensure that the active currency is set to false in the array so that there is no double
// counting during FreeCollateral
setActiveCurrency(accountContext, currencyId, false, Constants.ACTIVE_IN_BALANCES);
accountContext.bitmapCurrencyId = currencyId;

// Setting this is required to initialize the assets bitmap
Expand Down
6 changes: 6 additions & 0 deletions contracts/internal/valuation/FreeCollateral.sol
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ library FreeCollateral {
while (currencies != 0) {
bytes2 currencyBytes = bytes2(currencies);
uint16 currencyId = uint16(currencyBytes & Constants.UNMASK_FLAGS);
// Explicitly ensures that bitmap currency cannot be double counted
require(currencyId != accountContext.bitmapCurrencyId);

(int256 netLocalAssetValue, int256 nTokenBalance) =
_getCurrencyBalances(account, currencyBytes);
Expand Down Expand Up @@ -339,6 +341,8 @@ library FreeCollateral {
while (currencies != 0) {
bytes2 currencyBytes = bytes2(currencies);
uint16 currencyId = uint16(currencyBytes & Constants.UNMASK_FLAGS);
// Explicitly ensures that bitmap currency cannot be double counted
require(currencyId != accountContext.bitmapCurrencyId);
int256 nTokenBalance;
(netLocalAssetValues[netLocalIndex], nTokenBalance) = _getCurrencyBalances(
account,
Expand Down Expand Up @@ -464,6 +468,8 @@ library FreeCollateral {
bool setLiquidationFactors;
{
uint256 tempId = uint256(uint16(currencyBytes & Constants.UNMASK_FLAGS));
// Explicitly ensures that bitmap currency cannot be double counted
require(tempId != accountContext.bitmapCurrencyId);
setLiquidationFactors =
(tempId == localCurrencyId && collateralCurrencyId == 0) ||
tempId == collateralCurrencyId;
Expand Down
2 changes: 1 addition & 1 deletion contracts/mocks/MockAccountContextHandler.sol
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ contract MockAccountContextHandler {
uint256 blockTime
) external {
AccountContext memory accountContext = AccountContextHandler.getAccountContext(account);
accountContext.enableBitmapForAccount(account, currencyId, blockTime);
accountContext.enableBitmapForAccount(currencyId, blockTime);
accountContext.setAccountContext(account);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def test_exchange_rate_proportion(self, market, proportion):
)

# We do not allow proportion to exceed this max
if proportion > 990000000:
if proportion > 960000000:
assert not success
return

Expand Down
27 changes: 15 additions & 12 deletions tests/internal/test_account_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -440,15 +440,9 @@ def test_enable_bitmap_currency(self, accountContext, accounts):
assert context[0] != 0
assert context[3] == 1

accountContext.enableBitmapForAccount(accounts[0], 4, START_TIME)
context = accountContext.getAccountContext(accounts[0])
assert context[0] != 0
assert context[3] == 4

accountContext.enableBitmapForAccount(accounts[0], 0, START_TIME)
context = accountContext.getAccountContext(accounts[0])
assert context[0] != 0
assert context[3] == 0
with brownie.reverts("Cannot change bitmap"):
accountContext.enableBitmapForAccount(accounts[0], 4, START_TIME)
accountContext.enableBitmapForAccount(accounts[0], 0, START_TIME)

def test_enable_bitmap_currency_duplicate(self, accountContext, accounts):
# Ensure that only the bitmap currency is logged as a record for the active currency
Expand All @@ -463,14 +457,23 @@ def test_enable_bitmap_currency_duplicate(self, accountContext, accounts):
assert context[-1] == HexString(currencies_list_to_active_currency_bytes([]), "bytes18")

def test_fail_enable_bitmap_currency(self, accountContext, accounts):
with brownie.reverts("AC: invalid currency id"):
with brownie.reverts("Invalid currency id"):
accountContext.enableBitmapForAccount(accounts[0], 16384, START_TIME)
accountContext.enableBitmapForAccount(accounts[0], 0, START_TIME)

with brownie.reverts("AC: cannot have assets"):
with brownie.reverts("Cannot change bitmap"):
accountContext.setAccountContext((START_TIME, "0x00", 0, 1, "0x00"), accounts[0])
accountContext.enableBitmapForAccount(accounts[0], 1, START_TIME)

with brownie.reverts("Cannot have assets"):
accountContext.setAccountContext((START_TIME, "0x00", 1, 0, "0x00"), accounts[0])
accountContext.enableBitmapForAccount(accounts[0], 1, START_TIME)

with brownie.reverts("AC: cannot have assets"):
with brownie.reverts("Cannot change bitmap"):
accountContext.setAccountContext((START_TIME, "0x00", 0, 5, "0x00"), accounts[0])
accountContext.setAssetBitmap(accounts[0], 5, "0x1")
accountContext.enableBitmapForAccount(accounts[0], 1, START_TIME)

with brownie.reverts("Cannot have debt"):
accountContext.setAccountContext((START_TIME, "0x01", 0, 0, "0x4001"), accounts[0])
accountContext.enableBitmapForAccount(accounts[0], 1, START_TIME)