Skip to content
This repository has been archived by the owner on Mar 28, 2023. It is now read-only.

DepositLog refactor #537

Merged
merged 8 commits into from
Mar 27, 2020
Merged

DepositLog refactor #537

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8a9db61
Remove outdated comment
Mar 23, 2020
8ea92a6
Remove tbtcDepositToken from tbtcSystem
Mar 23, 2020
6837b39
Remove log return values
Mar 23, 2020
7aad202
Update tests
Mar 23, 2020
612fbf9
Merge branch 'master' into log-refactor
NicholasDotSol Mar 23, 2020
f1810be
Make setTbtcDepositToken internal
Mar 23, 2020
597cdfe
Merge branch 'log-refactor' of https://github.com/keep-network/tbtc i…
Mar 23, 2020
53e6030
Merge branch 'master' into log-refactor
Mar 27, 2020
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
214 changes: 110 additions & 104 deletions solidity/contracts/DepositLog.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,61 +104,53 @@ contract DepositLog {
// AUTH
//

/// @notice Checks if an address is an allowed logger
/// @notice Checks if an address is an allowed logger.
/// @dev checks tbtcDepositToken to see if the caller represents
/// an existing deposit.
/// We don't require this, so deposits are not bricked if the system borks
/// @param _caller The address of the calling contract
/// @return True if approved, otherwise false
/* solium-disable-next-line no-empty-blocks */
/// We don't require this, so deposits are not bricked if the system borks.
/// @param _caller The address of the calling contract.
/// @return True if approved, otherwise false.
function approvedToLog(address _caller) public view returns (bool) {
return tbtcDepositToken.exists(uint256(_caller));
}

/// @notice Sets the tbtcDepositToken contract.
/// @dev The contract is used by `approvedToLog` to check if the
/// caller is a Deposit contract. This should only be called once.
/// @param _tbtcDepositToken tbtcDepositToken contract.
function setTbtcDepositToken(TBTCDepositToken _tbtcDepositToken) public {
require(
address(tbtcDepositToken) == address(0),
"tbtcDepositToken is already set"
);
tbtcDepositToken = _tbtcDepositToken;
}

//
// Logging
//

/// @notice Fires a Created event
/// @dev We append the sender, which is the deposit contract that called
/// @param _keepAddress The address of the associated keep
/// @return True if successful, else revert
function logCreated(address _keepAddress) public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @notice Fires a Created event.
/// @dev We append the sender, which is the deposit contract that called.
/// @param _keepAddress The address of the associated keep.
/// @return True if successful, else revert.
function logCreated(address _keepAddress) public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit Created(msg.sender, _keepAddress, block.timestamp);
return true;
}

/// @notice Fires a RedemptionRequested event
/// @dev This is the only event without an explicit timestamp
/// @param _requester The ethereum address of the requester
/// @param _digest The calculated sighash digest
/// @param _utxoSize The size of the utxo in sat
/// @notice Fires a RedemptionRequested event.
/// @dev This is the only event without an explicit timestamp.
/// @param _requester The ethereum address of the requester.
/// @param _digest The calculated sighash digest.
/// @param _utxoSize The size of the utxo in sat.
/// @param _redeemerOutputScript The redeemer's length-prefixed output script.
/// @param _requestedFee The requester or bump-system specified fee
/// @param _outpoint The 36 byte outpoint
/// @return True if successful, else revert
/// @param _requestedFee The requester or bump-system specified fee.
/// @param _outpoint The 36 byte outpoint.
/// @return True if successful, else revert.
function logRedemptionRequested(
address _requester,
bytes32 _digest,
uint256 _utxoSize,
bytes memory _redeemerOutputScript,
uint256 _requestedFee,
bytes memory _outpoint
) public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
) public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit RedemptionRequested(
msg.sender,
_requester,
Expand All @@ -168,125 +160,139 @@ contract DepositLog {
_requestedFee,
_outpoint
);
return true;
}

/// @notice Fires a GotRedemptionSignature event
/// @dev We append the sender, which is the deposit contract that called
/// @param _digest signed digest
/// @param _r signature r value
/// @param _s signature s value
/// @return True if successful, else revert
/// @notice Fires a GotRedemptionSignature event.
/// @dev We append the sender, which is the deposit contract that called.
/// @param _digest signed digest.
/// @param _r signature r value.
/// @param _s signature s value.
function logGotRedemptionSignature(bytes32 _digest, bytes32 _r, bytes32 _s)
public
returns (bool)
{
if (!approvedToLog(msg.sender)) return false;
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit GotRedemptionSignature(
msg.sender,
_digest,
_r,
_s,
block.timestamp
);
return true;
}

/// @notice Fires a RegisteredPubkey event
/// @dev We append the sender, which is the deposit contract that called
/// returns false if not approved, to prevent accidentally halting Deposit
/// @return True if successful, else false
/// @notice Fires a RegisteredPubkey event.
/// @dev We append the sender, which is the deposit contract that called.
function logRegisteredPubkey(
bytes32 _signingGroupPubkeyX,
bytes32 _signingGroupPubkeyY
) public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
) public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit RegisteredPubkey(
msg.sender,
_signingGroupPubkeyX,
_signingGroupPubkeyY,
block.timestamp
);
return true;
}

/// @notice Fires a SetupFailed event
/// @dev We append the sender, which is the deposit contract that called
/// returns false if not approved, to prevent accidentally halting Deposit
/// @return True if successful, else false
function logSetupFailed() public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @notice Fires a SetupFailed event.
/// @dev We append the sender, which is the deposit contract that called.
function logSetupFailed() public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit SetupFailed(msg.sender, block.timestamp);
return true;
}

/// @notice Fires a FraudDuringSetup event
/// @dev We append the sender, which is the deposit contract that called
/// returns false if not approved, to prevent accidentally halting Deposit
/// @return True if successful, else false
function logFraudDuringSetup() public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @notice Fires a FraudDuringSetup event.
/// @dev We append the sender, which is the deposit contract that called.
function logFraudDuringSetup() public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit FraudDuringSetup(msg.sender, block.timestamp);
return true;
}

/// @notice Fires a Funded event
/// @dev We append the sender, which is the deposit contract that called
/// returns false if not approved, to prevent accidentally halting Deposit
/// @return True if successful, else false
function logFunded() public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @notice Fires a Funded event.
/// @dev We append the sender, which is the deposit contract that called.
function logFunded() public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit Funded(msg.sender, block.timestamp);
return true;
}

/// @notice Fires a CourtesyCalled event
/// @dev We append the sender, which is the deposit contract that called
/// returns false if not approved, to prevent accidentally halting Deposit
/// @return True if successful, else false
function logCourtesyCalled() public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @notice Fires a CourtesyCalled event.
/// @dev We append the sender, which is the deposit contract that called.
function logCourtesyCalled() public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit CourtesyCalled(msg.sender, block.timestamp);
return true;
}

/// @notice Fires a StartedLiquidation event
/// @dev We append the sender, which is the deposit contract that called
/// @param _wasFraud True if liquidating for fraud
/// @return True if successful, else revert
function logStartedLiquidation(bool _wasFraud) public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @notice Fires a StartedLiquidation event.
/// @dev We append the sender, which is the deposit contract that called.
/// @param _wasFraud True if liquidating for fraud.
function logStartedLiquidation(bool _wasFraud) public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit StartedLiquidation(msg.sender, _wasFraud, block.timestamp);
return true;
}

/// @notice Fires a Redeemed event
/// @dev We append the sender, which is the deposit contract that called
/// returns false if not approved, to prevent accidentally halting Deposit
/// @return True if successful, else false
function logRedeemed(bytes32 _txid) public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @dev We append the sender, which is the deposit contract that called.
function logRedeemed(bytes32 _txid) public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit Redeemed(msg.sender, _txid, block.timestamp);
return true;
}

/// @notice Fires a Liquidated event
/// @dev We append the sender, which is the deposit contract that called
/// returns false if not approved, to prevent accidentally halting Deposit
/// @return True if successful, else false
function logLiquidated() public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @dev We append the sender, which is the deposit contract that called.
function logLiquidated() public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit Liquidated(msg.sender, block.timestamp);
return true;
}

/// @notice Fires a ExitedCourtesyCall event
/// @dev We append the sender, which is the deposit contract that called
/// returns false if not approved, to prevent accidentally halting Deposit
/// @return True if successful, else false
function logExitedCourtesyCall() public returns (bool) {
if (!approvedToLog(msg.sender)) return false;
/// @dev We append the sender, which is the deposit contract that called.
function logExitedCourtesyCall() public {
require(
approvedToLog(msg.sender),
"Caller is not approved to log events"
);
emit ExitedCourtesyCall(msg.sender, block.timestamp);
return true;
}

/// @notice Sets the tbtcDepositToken contract.
/// @dev The contract is used by `approvedToLog` to check if the
/// caller is a Deposit contract. This should only be called once.
/// @param _tbtcDepositTokenAddress The address of the tbtcDepositToken.
function setTbtcDepositToken(TBTCDepositToken _tbtcDepositTokenAddress)
internal
{
require(
address(tbtcDepositToken) == address(0),
"tbtcDepositToken is already set"
);
tbtcDepositToken = _tbtcDepositTokenAddress;
}
}
2 changes: 0 additions & 2 deletions solidity/contracts/system/TBTCSystem.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog {
uint256 pausedTimestamp;
uint256 constant pausedDuration = 10 days;

TBTCDepositToken public tbtcDepositToken;
IBTCETHPriceFeed public priceFeed;
IBondedECDSAKeepVendor public keepVendor;
IRelay public relay;
Expand Down Expand Up @@ -102,7 +101,6 @@ contract TBTCSystem is Ownable, ITBTCSystem, DepositLog {
uint256 _keepSize
) external onlyOwner {
require(!_initialized, "already initialized");
tbtcDepositToken = _tbtcDepositToken;
keepVendor = _keepVendor;
_vendingMachine.setExternalAddresses(
_tbtcToken,
Expand Down
3 changes: 2 additions & 1 deletion solidity/test/DepositFactoryTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const {deployAndLinkAll} = require("./helpers/testDeployer.js")
const {contract, web3} = require("@openzeppelin/test-environment")
const {contract, web3, accounts} = require("@openzeppelin/test-environment")
const {states} = require("./helpers/utils.js")
const {BN, constants, expectRevert} = require("@openzeppelin/test-helpers")
const {ZERO_ADDRESS} = constants
Expand Down Expand Up @@ -174,6 +174,7 @@ describe("DepositFactory", async function() {
it("is not affected by state changes to master", async () => {
const keep = await ECDSAKeepStub.new()

await tbtcDepositToken.forceMint(accounts[0], testDeposit.address)
await testDeposit.createNewDeposit(
tbtcSystemStub.address,
tbtcToken.address,
Expand Down
8 changes: 8 additions & 0 deletions solidity/test/DepositUtilsTest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,10 @@ describe("DepositUtils", async function() {
beneficiary = accounts[2]

feeRebateToken.forceMint(beneficiary, web3.utils.toBN(testDeposit.address))
tbtcDepositToken.forceMint(
beneficiary,
web3.utils.toBN(testDeposit.address),
)

await testDeposit.createNewDeposit(
tbtcSystemStub.address,
Expand Down Expand Up @@ -283,6 +287,10 @@ describe("DepositUtils", async function() {

beneficiary = accounts[2]

tbtcDepositToken.forceMint(
beneficiary,
web3.utils.toBN(testDeposit.address),
)
feeRebateToken.forceMint(
beneficiary,
web3.utils.toBN(testDeposit.address),
Expand Down