From 951987d6fa0699658ebe4031ec00826b72045b8e Mon Sep 17 00:00:00 2001 From: Andrew Richardson Date: Fri, 17 Jan 2025 11:54:20 -0500 Subject: [PATCH] noto: ensure mint/unlock are properly protected when using hooks Signed-off-by: Andrew Richardson --- solidity/contracts/private/BondTracker.sol | 37 +++++++++++-------- .../contracts/private/NotoTrackerERC20.sol | 29 +++++++++++++-- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/solidity/contracts/private/BondTracker.sol b/solidity/contracts/private/BondTracker.sol index b0ab9d4fa..3bff7e45a 100644 --- a/solidity/contracts/private/BondTracker.sol +++ b/solidity/contracts/private/BondTracker.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 pragma solidity ^0.8.20; -import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; import {NotoTrackerERC20} from "./NotoTrackerERC20.sol"; import {InvestorList} from "./InvestorList.sol"; @@ -9,7 +8,7 @@ import {InvestorList} from "./InvestorList.sol"; * @title BondTracker * @dev Hook logic to model a simple bond lifecycle on top of Noto. */ -contract BondTracker is NotoTrackerERC20, Ownable { +contract BondTracker is NotoTrackerERC20 { enum Status { INITIALIZED, ISSUED, @@ -21,10 +20,16 @@ contract BondTracker is NotoTrackerERC20, Ownable { Status internal _status; address internal _publicTracker; address internal _issuer; + address internal _custodian; InvestorList public investorList; - modifier onlyIssuer() { - require(_msgSender() == _issuer, "Sender is not issuer"); + modifier onlyIssuer(address sender) { + require(sender == _issuer, "Sender is not bond issuer"); + _; + } + + modifier onlyCustodian(address sender) { + require(sender == _custodian, "Sender is not bond custodian"); _; } @@ -33,17 +38,18 @@ contract BondTracker is NotoTrackerERC20, Ownable { string memory symbol, address custodian, address publicTracker - ) NotoTrackerERC20(name, symbol) Ownable(custodian) { + ) NotoTrackerERC20(name, symbol) { _status = Status.INITIALIZED; _publicTracker = publicTracker; - _issuer = _msgSender(); + _issuer = msg.sender; + _custodian = custodian; investorList = new InvestorList(custodian); } function beginDistribution( uint256 discountPrice, uint256 minimumDenomination - ) external onlyOwner { + ) external onlyCustodian(msg.sender) { require(_status == Status.ISSUED, "Bond has not been issued"); _status = Status.DISTRIBUTION_STARTED; emit PenteExternalCall( @@ -56,7 +62,7 @@ contract BondTracker is NotoTrackerERC20, Ownable { ); } - function closeDistribution() external onlyOwner { + function closeDistribution() external onlyCustodian(msg.sender) { _status = Status.DISTRIBUTION_CLOSED; emit PenteExternalCall( _publicTracker, @@ -64,7 +70,7 @@ contract BondTracker is NotoTrackerERC20, Ownable { ); } - function setActive() external onlyIssuer { + function setActive() external onlyIssuer(msg.sender) { require( _status == Status.DISTRIBUTION_CLOSED, "Bond is not ready to be activated" @@ -106,9 +112,8 @@ contract BondTracker is NotoTrackerERC20, Ownable { uint256 amount, bytes calldata data, PreparedTransaction calldata prepared - ) external virtual override onlyOwner { - require(sender == _issuer, "Bond must be issued by issuer"); - require(to == owner(), "Bond must be issued to custodian"); + ) external virtual override onlyIssuer(sender) { + require(to == _custodian, "Bond must be issued to custodian"); require(_status == Status.INITIALIZED, "Bond has already been issued"); _onMint(sender, to, amount, data, prepared); @@ -127,11 +132,11 @@ contract BondTracker is NotoTrackerERC20, Ownable { uint256 amount, bytes calldata data, PreparedTransaction calldata prepared - ) external virtual override onlyOwner { + ) external virtual override onlySelf(sender, from) { _checkTransfer(sender, from, to, amount); _onTransfer(sender, from, to, amount, data, prepared); - if (_status == Status.DISTRIBUTION_STARTED && from == owner()) { + if (_status == Status.DISTRIBUTION_STARTED && from == _custodian) { emit PenteExternalCall( _publicTracker, abi.encodeWithSignature("onDistribute(uint256)", amount) @@ -145,7 +150,7 @@ contract BondTracker is NotoTrackerERC20, Ownable { UnlockRecipient[] calldata recipients, bytes calldata data, PreparedTransaction calldata prepared - ) external virtual override onlyOwner { + ) external virtual override onlyLockOwner(sender, lockId) { _checkTransfers(sender, _locks.ownerOf(lockId), recipients); _onUnlock(sender, lockId, recipients, data, prepared); } @@ -156,7 +161,7 @@ contract BondTracker is NotoTrackerERC20, Ownable { UnlockRecipient[] calldata recipients, bytes calldata data, PreparedTransaction calldata prepared - ) external virtual override onlyOwner { + ) external virtual override onlyLockOwner(sender, lockId) { _checkTransfers(sender, _locks.ownerOf(lockId), recipients); _onPrepareUnlock(sender, lockId, recipients, data, prepared); } diff --git a/solidity/contracts/private/NotoTrackerERC20.sol b/solidity/contracts/private/NotoTrackerERC20.sol index 25bd933e1..8196c9359 100644 --- a/solidity/contracts/private/NotoTrackerERC20.sol +++ b/solidity/contracts/private/NotoTrackerERC20.sol @@ -6,13 +6,34 @@ import {INotoHooks} from "../private/interfaces/INotoHooks.sol"; import {NotoLocks} from "./NotoLocks.sol"; /** + * @title NotoTrackerERC20 * @dev Example Noto hooks which track all Noto token movements on a private ERC20. */ contract NotoTrackerERC20 is INotoHooks, ERC20 { NotoLocks internal _locks; + address internal _notary; + + modifier onlyNotary(address sender) { + require(sender == _notary, "Sender is not the notary"); + _; + } + + modifier onlySelf(address sender, address from) { + require(sender == from, "Sender is not the from address"); + _; + } + + modifier onlyLockOwner(address sender, bytes32 lockId) { + require( + sender == _locks.ownerOf(lockId), + "Sender is not the lock owner" + ); + _; + } constructor(string memory name, string memory symbol) ERC20(name, symbol) { _locks = new NotoLocks(); + _notary = msg.sender; } function _onMint( @@ -93,7 +114,7 @@ contract NotoTrackerERC20 is INotoHooks, ERC20 { uint256 amount, bytes calldata data, PreparedTransaction calldata prepared - ) external virtual override { + ) external virtual override onlyNotary(sender) { _onMint(sender, to, amount, data, prepared); } @@ -104,7 +125,7 @@ contract NotoTrackerERC20 is INotoHooks, ERC20 { uint256 amount, bytes calldata data, PreparedTransaction calldata prepared - ) external virtual override { + ) external virtual override onlySelf(sender, from) { _onTransfer(sender, from, to, amount, data, prepared); } @@ -148,7 +169,7 @@ contract NotoTrackerERC20 is INotoHooks, ERC20 { UnlockRecipient[] calldata recipients, bytes calldata data, PreparedTransaction calldata prepared - ) external virtual override { + ) external virtual override onlyLockOwner(sender, lockId) { _onUnlock(sender, lockId, recipients, data, prepared); } @@ -158,7 +179,7 @@ contract NotoTrackerERC20 is INotoHooks, ERC20 { UnlockRecipient[] calldata recipients, bytes calldata data, PreparedTransaction calldata prepared - ) external virtual override { + ) external virtual override onlyLockOwner(sender, lockId) { _onPrepareUnlock(sender, lockId, recipients, data, prepared); }