Skip to content

Commit

Permalink
feat(collateral-contract): adds a authorized sender role to block sen…
Browse files Browse the repository at this point in the history 
…der actions for unauthorized addresses

Signed-off-by: Bryan Cole <[email protected]>
  • Loading branch information
@ColePBryan
ColePBryan committed Jul 19, 2023
1 parent 5710422 commit d89491e
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 17 deletions.
31 changes: 23 additions & 8 deletions src/Collateral.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ pragma solidity ^0.8.18;
import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import {AccessControlDefaultAdminRules} from "@openzeppelin/contracts/access/AccessControlDefaultAdminRules.sol";
import {TAPVerifier} from "./TAPVerifier.sol";
import {AllocationIDTracker} from "./AllocationIDTracker.sol";
import {IStaking} from "./IStaking.sol";
Expand All @@ -20,7 +21,7 @@ import {IStaking} from "./IStaking.sol";
* @notice This contract uses the `TAPVerifier` contract for recovering signer addresses
* from `RAVs`.
*/
contract Collateral {
contract Collateral is AccessControlDefaultAdminRules {
using SafeERC20 for IERC20;

struct CollateralAccount {
Expand All @@ -34,6 +35,9 @@ contract Collateral {
uint256 thawEndTimestamp; // Block number at which thawing period ends (zero if not thawing)
}

// Role for depositing collateral
bytes32 public constant AUTHORIZED_SENDER = keccak256("AUTHORIZED_SENDER");

// Stores how much collateral each sender has deposited for each receiver, as well as thawing information
mapping(address sender => mapping(address reciever => CollateralAccount collateralAccount))
private collateralAccounts;
Expand Down Expand Up @@ -160,7 +164,7 @@ contract Collateral {
address allocationIDTracker_,
uint256 withdrawCollateralThawingPeriod_,
uint256 revokeSignerThawingPeriod_
) {
) AccessControlDefaultAdminRules(3 days, msg.sender) {
collateralToken = IERC20(collateralToken_);
staking = IStaking(staking_);
tapVerifier = TAPVerifier(tapVerifier_);
Expand All @@ -184,7 +188,10 @@ contract Collateral {
* @notice The collateral must be approved for transfer by the sender.
* @notice REVERT: this function will revert if the collateral transfer fails.
*/
function deposit(address receiver, uint256 amount) external {
function deposit(
address receiver,
uint256 amount
) external onlyRole(AUTHORIZED_SENDER) {
collateralAccounts[msg.sender][receiver].balance += amount;
collateralToken.safeTransferFrom(msg.sender, address(this), amount);
emit Deposit(msg.sender, receiver, amount);
Expand All @@ -198,7 +205,10 @@ contract Collateral {
* - InsufficientCollateral: if the sender receiver collateral account does
* not have enough collateral (greater than `amount`)
*/
function thaw(address receiver, uint256 amount) external {
function thaw(
address receiver,
uint256 amount
) external onlyRole(AUTHORIZED_SENDER) {
CollateralAccount storage account = collateralAccounts[msg.sender][
receiver
];
Expand Down Expand Up @@ -236,7 +246,7 @@ contract Collateral {
* - CollateralStillThawing: ThawEndTimestamp has not been reached
* for collateral currently thawing
*/
function withdraw(address receiver) external {
function withdraw(address receiver) external onlyRole(AUTHORIZED_SENDER) {
CollateralAccount storage account = collateralAccounts[msg.sender][
receiver
];
Expand Down Expand Up @@ -273,7 +283,10 @@ contract Collateral {
* - SignerAlreadyAuthorized: Signer is currently authorized for a sender
* - InvalidSignerProof: The provided signer proof is invalid
*/
function authorizeSigner(address signer, bytes calldata proof) external {
function authorizeSigner(
address signer,
bytes calldata proof
) external onlyRole(AUTHORIZED_SENDER) {
if (authorizedSigners[signer].sender != address(0)) {
revert SignerAlreadyAuthorized(
signer,
Expand All @@ -295,7 +308,7 @@ contract Collateral {
* - SignerNotAuthorizedBySender: The provided signer is either not authorized or
* authorized by a different sender
*/
function thawSigner(address signer) external {
function thawSigner(address signer) external onlyRole(AUTHORIZED_SENDER) {
SenderAuthorization storage authorization = authorizedSigners[signer];

if (authorization.sender != msg.sender) {
Expand Down Expand Up @@ -325,7 +338,9 @@ contract Collateral {
* - SignerStillThawing: ThawEndTimestamp has not been reached
* for provided signer
*/
function revokeAuthorizedSigner(address signer) external {
function revokeAuthorizedSigner(
address signer
) external onlyRole(AUTHORIZED_SENDER) {
SenderAuthorization storage authorization = authorizedSigners[signer];

if (authorization.sender != msg.sender) {
Expand Down
50 changes: 41 additions & 9 deletions test/Collateral.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pragma solidity ^0.8.18;

import "forge-std/Test.sol";
import {ECDSA} from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol";
import {Strings} from "@openzeppelin/contracts/utils/Strings.sol";
import {TAPVerifier} from "../src/TAPVerifier.sol";
import {Collateral} from "../src/Collateral.sol";
import {MockERC20Token} from "./MockERC20Token.sol";
Expand Down Expand Up @@ -88,7 +89,7 @@ contract CollateralContractTest is Test {
}

function testDepositFunds() public {
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT);
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT, true);

vm.prank(SENDER_ADDRESS);
uint256 depositedAmount = collateralContract.getCollateralAmount(SENDER_ADDRESS, receiverAddress);
Expand All @@ -97,7 +98,7 @@ contract CollateralContractTest is Test {
}

function testWithdrawFundsAfterFreezePeriod() public {
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT);
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT, true);

// Sets msg.sender address for next contract calls until stop is called
vm.startPrank(SENDER_ADDRESS);
Expand All @@ -121,7 +122,7 @@ contract CollateralContractTest is Test {
}

function testRedeemRAVSignedByAuthorizedSigner() public {
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT);
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT, true);
uint256 remainingCollateral = collateralContract.getCollateralAmount(SENDER_ADDRESS, receiverAddress);
assertEq(remainingCollateral, COLLATERAL_AMOUNT, "Incorrect remaining collateral");

Expand Down Expand Up @@ -160,14 +161,14 @@ contract CollateralContractTest is Test {
}

function testGetCollateralAmount() public {
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT);
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT, true);

uint256 depositedAmount = collateralContract.getCollateralAmount(SENDER_ADDRESS, receiverAddress);
assertEq(depositedAmount, COLLATERAL_AMOUNT, "Incorrect deposited amount");
}

function testMultipleThawRequests() public {
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT);
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT, true);
uint256 partialCollateralAmount = COLLATERAL_AMOUNT / 10;
uint256 partialFreezePeriod = WITHDRAW_COLLATERAL_FREEZE_PERIOD / 10;
uint256 expectedThawEnd = 0;
Expand Down Expand Up @@ -205,7 +206,7 @@ contract CollateralContractTest is Test {

// test that the contract reverts when allocation ID is used more than once
function testRevertOnDuplicateAllocationID() public {
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT);
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT, true);
uint256 remainingCollateral = collateralContract.getCollateralAmount(SENDER_ADDRESS, receiverAddress);
assertEq(remainingCollateral, COLLATERAL_AMOUNT, "Incorrect remaining collateral");

Expand Down Expand Up @@ -263,7 +264,7 @@ contract CollateralContractTest is Test {
// create additional sender address to test that the contract does not revert when redeeming same allocation ID with a different sender
address secondSenderAddress = address(0xa789);
assert(mockERC20.transfer(secondSenderAddress, 10000000));
depositCollateral(secondSenderAddress, receiverAddress, COLLATERAL_AMOUNT);
depositCollateral(secondSenderAddress, receiverAddress, COLLATERAL_AMOUNT, true);

// should not revert when redeeming same allocationID with a different sender
authorizeSignerWithProof(secondSenderAddress, authorizedSignerPrivateKeys[1], authorizedsigners[1]);
Expand Down Expand Up @@ -291,7 +292,7 @@ contract CollateralContractTest is Test {
}

function testRevokeAuthorizedSigner() public {
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT);
depositCollateral(SENDER_ADDRESS, receiverAddress, COLLATERAL_AMOUNT, true);

authorizeSignerWithProof(SENDER_ADDRESS, authorizedSignerPrivateKeys[0], authorizedsigners[0]);
vm.prank(SENDER_ADDRESS);
Expand Down Expand Up @@ -336,6 +337,34 @@ contract CollateralContractTest is Test {
);
}

function testAccessControlledFunctions() public {
string memory expectedErrorMessage = string(
abi.encodePacked(
"AccessControl: account ",
Strings.toHexString(SENDER_ADDRESS),
" is missing role ",
Strings.toHexString(uint256(collateralContract.AUTHORIZED_SENDER()), 32)
)
);
// Call functions as sender (who currently has no role and is not authorized to call these functions)
vm.startPrank(SENDER_ADDRESS);

vm.expectRevert(bytes(expectedErrorMessage));
collateralContract.deposit(receiverAddress, 10);
vm.expectRevert(bytes(expectedErrorMessage));
collateralContract.thaw(receiverAddress, 10);
vm.expectRevert(bytes(expectedErrorMessage));
collateralContract.withdraw(receiverAddress);
vm.expectRevert(bytes(expectedErrorMessage));
collateralContract.authorizeSigner(authorizedsigners[0], bytes("test"));
vm.expectRevert(bytes(expectedErrorMessage));
collateralContract.thawSigner(authorizedsigners[0]);
vm.expectRevert(bytes(expectedErrorMessage));
collateralContract.revokeAuthorizedSigner(authorizedsigners[0]);

vm.stopPrank();
}

function authorizeSignerWithProof(address sender, uint256 signerPivateKey, address signer) private {
bytes memory authSignerAuthorizesSenderProof = createAuthorizedSignerProof(sender, signerPivateKey);

Expand Down Expand Up @@ -367,7 +396,10 @@ contract CollateralContractTest is Test {
return abi.encodePacked(r, s, v);
}

function depositCollateral(address sender, address receiver, uint256 amount) public {
function depositCollateral(address sender, address receiver, uint256 amount, bool grantRole) public {
if( grantRole && !collateralContract.hasRole(collateralContract.AUTHORIZED_SENDER(), sender) ) {
collateralContract.grantRole(collateralContract.AUTHORIZED_SENDER(), sender);
}
// Sets msg.sender address for next contract calls until stop is called
vm.startPrank(sender);
// Approve the collateral contract to transfer tokens from the sender
Expand Down

0 comments on commit d89491e

Please sign in to comment.