Skip to content

Commit

Permalink
feat(bootstrap): add reentrancy guards
Browse files Browse the repository at this point in the history
  • Loading branch information
MaxMustermann2 committed Jun 11, 2024
1 parent 267b46b commit b9409a5
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 25 deletions.
76 changes: 51 additions & 25 deletions src/core/Bootstrap.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {ITransparentUpgradeableProxy} from
import {OwnableUpgradeable} from "@openzeppelin-upgradeable/contracts/access/OwnableUpgradeable.sol";
import {Initializable} from "@openzeppelin-upgradeable/contracts/proxy/utils/Initializable.sol";
import {PausableUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/PausableUpgradeable.sol";
import {ReentrancyGuardUpgradeable} from "@openzeppelin-upgradeable/contracts/utils/ReentrancyGuardUpgradeable.sol";
import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol";

import {OAppCoreUpgradeable} from "../lzApp/OAppCoreUpgradeable.sol";
Expand All @@ -28,6 +29,7 @@ contract Bootstrap is
Initializable,
PausableUpgradeable,
OwnableUpgradeable,
ReentrancyGuardUpgradeable,
ILSTRestakingController,
IOperatorRegistry,
BootstrapLzReceiver
Expand Down Expand Up @@ -156,7 +158,7 @@ contract Bootstrap is
}

// implementation of ITokenWhitelister
function addWhitelistToken(address _token) public override beforeLocked onlyOwner whenNotPaused {
function addWhitelistToken(address _token) public override beforeLocked onlyOwner whenNotPaused nonReentrant {
super.addWhitelistToken(_token);
}

Expand Down Expand Up @@ -314,26 +316,32 @@ contract Bootstrap is
whenNotPaused
isTokenWhitelisted(token)
isValidAmount(amount)
nonReentrant // interacts with Vault
{
_deposit(msg.sender, token, amount);
}

// _deposit is the internal function that does the work
function _deposit(address depositor, address token, uint256 amount) internal {
IVault vault = _getVault(token);
vault.deposit(msg.sender, amount);
vault.deposit(depositor, amount);

if (!isDepositor[msg.sender]) {
isDepositor[msg.sender] = true;
depositors.push(msg.sender);
if (!isDepositor[depositor]) {
isDepositor[depositor] = true;
depositors.push(depositor);
}

// staker_asset.go duplicate here. the duplication is required (and not simply inferred
// from vault) because the vault is not altered by the gateway in response to
// delegations or undelegations. hence, this is not something we can do either.
totalDepositAmounts[msg.sender][token] += amount;
withdrawableAmounts[msg.sender][token] += amount;
totalDepositAmounts[depositor][token] += amount;
withdrawableAmounts[depositor][token] += amount;
depositsByToken[token] += amount;

// afterReceiveDepositResponse stores the TotalDepositAmount in the principle.
vault.updatePrincipleBalance(msg.sender, totalDepositAmounts[msg.sender][token]);
vault.updatePrincipleBalance(depositor, totalDepositAmounts[depositor][token]);

emit DepositResult(true, token, msg.sender, amount);
emit DepositResult(true, token, depositor, amount);
}

// implementation of ILSTRestakingController
Expand All @@ -346,24 +354,30 @@ contract Bootstrap is
whenNotPaused
isTokenWhitelisted(token)
isValidAmount(amount)
nonReentrant // interacts with Vault
{
_withdraw(msg.sender, token, amount);
}

// _withdraw is the internal function that does the actual work.
function _withdraw(address user, address token, uint256 amount) internal {
IVault vault = _getVault(token);

uint256 deposited = totalDepositAmounts[msg.sender][token];
uint256 deposited = totalDepositAmounts[user][token];
require(deposited >= amount, "Bootstrap: insufficient deposited balance");
uint256 withdrawable = withdrawableAmounts[msg.sender][token];
uint256 withdrawable = withdrawableAmounts[user][token];
require(withdrawable >= amount, "Bootstrap: insufficient withdrawable balance");

// when the withdraw precompile is called, it does these things.
totalDepositAmounts[msg.sender][token] -= amount;
withdrawableAmounts[msg.sender][token] -= amount;
totalDepositAmounts[user][token] -= amount;
withdrawableAmounts[user][token] -= amount;
depositsByToken[token] -= amount;

// afterReceiveWithdrawPrincipleResponse
vault.updatePrincipleBalance(msg.sender, totalDepositAmounts[msg.sender][token]);
vault.updateWithdrawableBalance(msg.sender, amount, 0);
vault.updatePrincipleBalance(user, totalDepositAmounts[user][token]);
vault.updateWithdrawableBalance(user, amount, 0);

emit WithdrawPrincipleResult(true, token, msg.sender, amount);
emit WithdrawPrincipleResult(true, token, user, amount);
}

// implementation of ILSTRestakingController
Expand All @@ -380,6 +394,7 @@ contract Bootstrap is
whenNotPaused
isTokenWhitelisted(token)
isValidAmount(amount)
nonReentrant // because it interacts with vault
{
IVault vault = _getVault(token);
vault.withdraw(msg.sender, recipient, amount);
Expand All @@ -395,7 +410,12 @@ contract Bootstrap is
isTokenWhitelisted(token)
isValidAmount(amount)
isValidBech32Address(operator)
// does not need a reentrancy guard
{
_delegateTo(msg.sender, operator, token, amount);
}

function _delegateTo(address user, string calldata operator, address token, uint256 amount) internal {
require(msg.value == 0, "Bootstrap: no ether required for delegation");
// check that operator is registered
require(bytes(operators[operator].name).length != 0, "Operator does not exist");
Expand All @@ -404,11 +424,11 @@ contract Bootstrap is
// now check amounts.
uint256 withdrawable = withdrawableAmounts[msg.sender][token];
require(withdrawable >= amount, "Bootstrap: insufficient withdrawable balance");
delegations[msg.sender][operator][token] += amount;
delegations[user][operator][token] += amount;
delegationsByOperator[operator][token] += amount;
withdrawableAmounts[msg.sender][token] -= amount;
withdrawableAmounts[user][token] -= amount;

emit DelegateResult(true, msg.sender, operator, token, amount);
emit DelegateResult(true, user, operator, token, amount);
}

// implementation of ILSTRestakingController
Expand All @@ -421,21 +441,26 @@ contract Bootstrap is
isTokenWhitelisted(token)
isValidAmount(amount)
isValidBech32Address(operator)
// does not need a reentrancy guard
{
_undelegateFrom(msg.sender, operator, token, amount);
}

function _undelegateFrom(address user, string calldata operator, address token, uint256 amount) internal {
require(msg.value == 0, "Bootstrap: no ether required for undelegation");
// check that operator is registered
require(bytes(operators[operator].name).length != 0, "Operator does not exist");
// operator can't be frozen and amount can't be negative
// asset validity has been checked.
// now check amounts.
uint256 delegated = delegations[msg.sender][operator][token];
uint256 delegated = delegations[user][operator][token];
require(delegated >= amount, "Bootstrap: insufficient delegated balance");
// the undelegation is released immediately since it is not at stake yet.
delegations[msg.sender][operator][token] -= amount;
delegations[user][operator][token] -= amount;
delegationsByOperator[operator][token] -= amount;
withdrawableAmounts[msg.sender][token] += amount;
withdrawableAmounts[user][token] += amount;

emit UndelegateResult(true, msg.sender, operator, token, amount);
emit UndelegateResult(true, user, operator, token, amount);
}

// implementation of ILSTRestakingController
Expand All @@ -448,9 +473,10 @@ contract Bootstrap is
isTokenWhitelisted(token)
isValidAmount(amount)
isValidBech32Address(operator)
nonReentrant // because it interacts with vault in deposit
{
this.deposit(token, amount);
this.delegateTo(operator, token, amount);
_deposit(msg.sender, token, amount);
_delegateTo(msg.sender, operator, token, amount);
}

/**
Expand Down
2 changes: 2 additions & 0 deletions src/core/ExocoreGateway.sol
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ contract ExocoreGateway is
// while some of the code from requestDeposit and requestDelegateTo is duplicated here,
// it is done intentionally to work around Solidity's limitations with regards to
// function calls, error handling and indexing the return data of memory type.
// for example, you cannot index a bytes memory result from the requestDepositTo call,
// if you were to modify it to return bytes and then process them here.

(bool success, uint256 updatedBalance) = DEPOSIT_CONTRACT.depositTo(srcChainId, token, depositor, amount);
if (!success) {
Expand Down
18 changes: 18 additions & 0 deletions test/foundry/Bootstrap.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,24 @@ contract BootstrapTest is Test {
vm.stopPrank();
}

function test04_DepositThenDelegate() public {
// since deposit and delegate are already tested, we will just do a simple success
// check here to ensure the reentrancy modifier works.
test03_RegisterOperator();
vm.startPrank(addrs[0]);
IVault vault = IVault(bootstrap.tokenToVault(address(myToken)));
myToken.approve(address(vault), amounts[0]);
bootstrap.depositThenDelegateTo(address(myToken), amounts[0], "exo13hasr43vvq8v44xpzh0l6yuym4kca98f87j7ac");
vm.stopPrank();

uint256 deposited = bootstrap.totalDepositAmounts(addrs[0], address(myToken));
assertTrue(deposited == amounts[0]);

uint256 delegated = bootstrap.delegations(addrs[0], "exo13hasr43vvq8v44xpzh0l6yuym4kca98f87j7ac",
address(myToken));
assertTrue(delegated == amounts[0]);
}

function test05_ReplaceKey() public {
IOperatorRegistry.Commission memory commission = IOperatorRegistry.Commission(0, 1e18, 1e18);
// Register operator
Expand Down

0 comments on commit b9409a5

Please sign in to comment.