-
Notifications
You must be signed in to change notification settings - Fork 32
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
solhint warning fixes [SLT-287] #3207
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,49 +31,14 @@ | |
|
|
||
| /// @dev to prevent replays | ||
| uint256 public nonce; | ||
|
|
||
| // @dev the block the contract was deployed at | ||
| uint256 public immutable deployBlock; | ||
|
|
||
| function bridgeStatuses(bytes32 transactionId) public view returns (BridgeStatus status) { | ||
| return bridgeTxDetails[transactionId].status; | ||
| } | ||
|
|
||
| function bridgeProofs(bytes32 transactionId) public view returns (uint96 timestamp, address relayer) { | ||
| timestamp = bridgeTxDetails[transactionId].proofBlockTimestamp; | ||
| relayer = bridgeTxDetails[transactionId].proofRelayer; | ||
| } | ||
|
|
||
| constructor(address _owner) Admin(_owner) { | ||
| deployBlock = block.number; | ||
| } | ||
|
|
||
| /// @notice Pulls a requested token from the user to the requested recipient. | ||
| /// @dev Be careful of re-entrancy issues when msg.value > 0 and recipient != address(this) | ||
| function _pullToken(address recipient, address token, uint256 amount) internal returns (uint256 amountPulled) { | ||
| if (token != UniversalTokenLib.ETH_ADDRESS) { | ||
| token.assertIsContract(); | ||
| // Record token balance before transfer | ||
| amountPulled = IERC20(token).balanceOf(recipient); | ||
| // Token needs to be pulled only if msg.value is zero | ||
| // This way user can specify WETH as the origin asset | ||
| IERC20(token).safeTransferFrom(msg.sender, recipient, amount); | ||
| // Use the difference between the recorded balance and the current balance as the amountPulled | ||
| amountPulled = IERC20(token).balanceOf(recipient) - amountPulled; | ||
| } else { | ||
| // Otherwise, we need to check that ETH amount matches msg.value | ||
| if (amount != msg.value) revert MsgValueIncorrect(); | ||
| // Transfer value to recipient if not this address | ||
| if (recipient != address(this)) token.universalTransfer(recipient, amount); | ||
| // We will forward msg.value in the external call later, if recipient is not this contract | ||
| amountPulled = msg.value; | ||
| } | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function getBridgeTransaction(bytes memory request) public pure returns (BridgeTransaction memory) { | ||
| return abi.decode(request, (BridgeTransaction)); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function bridge(BridgeParams memory params) external payable { | ||
| // check bridge params | ||
|
|
@@ -127,8 +92,69 @@ | |
|
|
||
| /// @inheritdoc IFastBridge | ||
| function relay(bytes memory request) external payable { | ||
| relay(request, msg.sender); | ||
| relay({request: request, relayer: msg.sender}); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function prove(bytes memory request, bytes32 destTxHash) external { | ||
| prove({transactionId: keccak256(request), destTxHash: destTxHash, relayer: msg.sender}); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridgeV2 | ||
| function claim(bytes memory request) external { | ||
| claim({request: request, to: address(0)}); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function dispute(bytes32 transactionId) external onlyRole(GUARD_ROLE) { | ||
| if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); | ||
| if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD) { | ||
| revert DisputePeriodPassed(); | ||
| } | ||
|
|
||
| // @dev relayer gets slashed effectively if dest relay has gone thru | ||
| bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED; | ||
| bridgeTxDetails[transactionId].proofRelayer = address(0); | ||
| bridgeTxDetails[transactionId].proofBlockTimestamp = 0; | ||
| bridgeTxDetails[transactionId].proofBlockNumber = 0; | ||
|
|
||
| emit BridgeProofDisputed(transactionId, msg.sender); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function refund(bytes memory request) external { | ||
| bytes32 transactionId = keccak256(request); | ||
|
|
||
| BridgeTransaction memory transaction = getBridgeTransaction(request); | ||
|
|
||
| if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); | ||
|
|
||
| if (hasRole(REFUNDER_ROLE, msg.sender)) { | ||
| // Refunder can refund if deadline has passed | ||
| if (block.timestamp <= transaction.deadline) revert DeadlineNotExceeded(); | ||
| } else { | ||
| // Permissionless refund is allowed after REFUND_DELAY | ||
| if (block.timestamp <= transaction.deadline + REFUND_DELAY) revert DeadlineNotExceeded(); | ||
| } | ||
|
|
||
| // if all checks passed, set to REFUNDED status | ||
| bridgeTxDetails[transactionId].status = BridgeStatus.REFUNDED; | ||
|
|
||
| // transfer origin collateral back to original sender | ||
| address to = transaction.originSender; | ||
| address token = transaction.originToken; | ||
| uint256 amount = transaction.originAmount + transaction.originFeeAmount; | ||
| token.universalTransfer(to, amount); | ||
|
|
||
| emit BridgeDepositRefunded(transactionId, to, token, amount); | ||
| } | ||
|
Comment on lines
+125
to
+150
Check notice Code scanning / Slither Reentrancy vulnerabilities Low
Reentrancy in FastBridgeV2.refund(bytes):
External calls: - Address.sendValue(address(to),amount) - IERC20(token).safeTransfer(to,amount) Event emitted after the call(s): - BridgeDepositRefunded(transactionId,transaction.originSender,transaction.originToken,amount)
Comment on lines
+125
to
+150
Check notice Code scanning / Slither Block timestamp Low
FastBridgeV2.refund(bytes) uses timestamp for comparisons
Dangerous comparisons: - block.timestamp <= transaction.deadline - block.timestamp <= transaction.deadline + REFUND_DELAY |
||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function canClaim(bytes32 transactionId, address relayer) external view returns (bool) { | ||
| if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); | ||
| if (bridgeTxDetails[transactionId].proofRelayer != relayer) revert SenderIncorrect(); | ||
| return _timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD; | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridgeV2 | ||
| function relay(bytes memory request, address relayer) public payable { | ||
|
|
@@ -178,18 +204,6 @@ | |
| ); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridgeV2 | ||
| function bridgeRelays(bytes32 transactionId) public view returns (bool) { | ||
| // has this transactionId been relayed? | ||
| return bridgeRelayDetails[transactionId].relayer != address(0); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function prove(bytes memory request, bytes32 destTxHash) external { | ||
| bytes32 transactionId = keccak256(request); | ||
| prove(transactionId, destTxHash, msg.sender); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridgeV2 | ||
| function prove(bytes32 transactionId, bytes32 destTxHash, address relayer) public onlyRole(RELAYER_ROLE) { | ||
| // update bridge tx status given proof provided | ||
|
|
@@ -202,30 +216,6 @@ | |
| emit BridgeProofProvided(transactionId, relayer, destTxHash); | ||
| } | ||
|
|
||
| /// @notice Calculates time since proof submitted | ||
| /// @dev proof.timestamp stores casted uint40(block.timestamp) block timestamps for gas optimization | ||
| /// _timeSince(proof) can accomodate rollover case when block.timestamp > type(uint40).max but | ||
| /// proof.timestamp < type(uint40).max via unchecked statement | ||
| /// @param proofBlockTimestamp The bridge proof block timestamp | ||
| /// @return delta Time delta since proof submitted | ||
| function _timeSince(uint40 proofBlockTimestamp) internal view returns (uint256 delta) { | ||
| unchecked { | ||
| delta = uint40(block.timestamp) - proofBlockTimestamp; | ||
| } | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function canClaim(bytes32 transactionId, address relayer) external view returns (bool) { | ||
| if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); | ||
| if (bridgeTxDetails[transactionId].proofRelayer != relayer) revert SenderIncorrect(); | ||
| return _timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD; | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridgeV2 | ||
| function claim(bytes memory request) external { | ||
| claim(request, address(0)); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function claim(bytes memory request, address to) public { | ||
| bytes32 transactionId = keccak256(request); | ||
|
|
@@ -258,47 +248,57 @@ | |
| emit BridgeDepositClaimed(transactionId, bridgeTxDetails[transactionId].proofRelayer, to, token, amount); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function dispute(bytes32 transactionId) external onlyRole(GUARD_ROLE) { | ||
| if (bridgeTxDetails[transactionId].status != BridgeStatus.RELAYER_PROVED) revert StatusIncorrect(); | ||
| if (_timeSince(bridgeTxDetails[transactionId].proofBlockTimestamp) > DISPUTE_PERIOD) { | ||
| revert DisputePeriodPassed(); | ||
| } | ||
| function bridgeStatuses(bytes32 transactionId) public view returns (BridgeStatus status) { | ||
| return bridgeTxDetails[transactionId].status; | ||
| } | ||
|
|
||
| // @dev relayer gets slashed effectively if dest relay has gone thru | ||
| bridgeTxDetails[transactionId].status = BridgeStatus.REQUESTED; | ||
| bridgeTxDetails[transactionId].proofRelayer = address(0); | ||
| bridgeTxDetails[transactionId].proofBlockTimestamp = 0; | ||
| bridgeTxDetails[transactionId].proofBlockNumber = 0; | ||
| function bridgeProofs(bytes32 transactionId) public view returns (uint96 timestamp, address relayer) { | ||
| timestamp = bridgeTxDetails[transactionId].proofBlockTimestamp; | ||
| relayer = bridgeTxDetails[transactionId].proofRelayer; | ||
| } | ||
|
|
||
| emit BridgeProofDisputed(transactionId, msg.sender); | ||
| /// @inheritdoc IFastBridgeV2 | ||
| function bridgeRelays(bytes32 transactionId) public view returns (bool) { | ||
| // has this transactionId been relayed? | ||
| return bridgeRelayDetails[transactionId].relayer != address(0); | ||
| } | ||
|
|
||
| /// @inheritdoc IFastBridge | ||
| function refund(bytes memory request) external { | ||
| bytes32 transactionId = keccak256(request); | ||
|
|
||
| BridgeTransaction memory transaction = getBridgeTransaction(request); | ||
|
|
||
| if (bridgeTxDetails[transactionId].status != BridgeStatus.REQUESTED) revert StatusIncorrect(); | ||
| function getBridgeTransaction(bytes memory request) public pure returns (BridgeTransaction memory) { | ||
| return abi.decode(request, (BridgeTransaction)); | ||
| } | ||
|
|
||
| if (hasRole(REFUNDER_ROLE, msg.sender)) { | ||
| // Refunder can refund if deadline has passed | ||
| if (block.timestamp <= transaction.deadline) revert DeadlineNotExceeded(); | ||
| /// @notice Pulls a requested token from the user to the requested recipient. | ||
| /// @dev Be careful of re-entrancy issues when msg.value > 0 and recipient != address(this) | ||
| function _pullToken(address recipient, address token, uint256 amount) internal returns (uint256 amountPulled) { | ||
| if (token != UniversalTokenLib.ETH_ADDRESS) { | ||
| token.assertIsContract(); | ||
| // Record token balance before transfer | ||
| amountPulled = IERC20(token).balanceOf(recipient); | ||
| // Token needs to be pulled only if msg.value is zero | ||
| // This way user can specify WETH as the origin asset | ||
| IERC20(token).safeTransferFrom(msg.sender, recipient, amount); | ||
| // Use the difference between the recorded balance and the current balance as the amountPulled | ||
| amountPulled = IERC20(token).balanceOf(recipient) - amountPulled; | ||
| } else { | ||
| // Permissionless refund is allowed after REFUND_DELAY | ||
| if (block.timestamp <= transaction.deadline + REFUND_DELAY) revert DeadlineNotExceeded(); | ||
| // Otherwise, we need to check that ETH amount matches msg.value | ||
| if (amount != msg.value) revert MsgValueIncorrect(); | ||
| // Transfer value to recipient if not this address | ||
| if (recipient != address(this)) token.universalTransfer(recipient, amount); | ||
| // We will forward msg.value in the external call later, if recipient is not this contract | ||
| amountPulled = msg.value; | ||
| } | ||
| } | ||
|
|
||
| // if all checks passed, set to REFUNDED status | ||
| bridgeTxDetails[transactionId].status = BridgeStatus.REFUNDED; | ||
|
|
||
| // transfer origin collateral back to original sender | ||
| address to = transaction.originSender; | ||
| address token = transaction.originToken; | ||
| uint256 amount = transaction.originAmount + transaction.originFeeAmount; | ||
| token.universalTransfer(to, amount); | ||
|
|
||
| emit BridgeDepositRefunded(transactionId, to, token, amount); | ||
| /// @notice Calculates time since proof submitted | ||
| /// @dev proof.timestamp stores casted uint40(block.timestamp) block timestamps for gas optimization | ||
| /// _timeSince(proof) can accomodate rollover case when block.timestamp > type(uint40).max but | ||
| /// proof.timestamp < type(uint40).max via unchecked statement | ||
| /// @param proofBlockTimestamp The bridge proof block timestamp | ||
| /// @return delta Time delta since proof submitted | ||
| function _timeSince(uint40 proofBlockTimestamp) internal view returns (uint256 delta) { | ||
| unchecked { | ||
| delta = uint40(block.timestamp) - proofBlockTimestamp; | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Check notice
Code scanning / Slither
Block timestamp Low