Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Unilateral pause implementation #402

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Unilateral pause implementation #402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
52f810a
Unilateral pause implementation
Apr 11, 2024
1d9dcef
Forge fmt
Apr 11, 2024
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
51 changes: 49 additions & 2 deletions evm/src/NttManager/ManagerBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import "../interfaces/ITransceiver.sol";
import "../interfaces/IManagerBase.sol";

import "./TransceiverRegistry.sol";
import "forge-std/console.sol"; // TODO - remove this - MAX

abstract contract ManagerBase is
IManagerBase,
Expand Down Expand Up @@ -42,6 +43,27 @@ abstract contract ManagerBase is
deployer = msg.sender;
}

modifier inboundNotPaused() {
if (_getUnilateralPauseStorage().inbound) {
revert InboundPaused();
}
_;
}

modifier outboundNotPaused() {
if (_getUnilateralPauseStorage().outbound) {
revert OutboundPaused();
}
_;
}

modifier outboundWhenPaused() {
if (_getUnilateralPauseStorage().outbound == false) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit pick but prefer ! vs == false

revert NotPausedForUpdate();
}
_;
}

function _migrate() internal virtual override {
_checkThresholdInvariants();
_checkTransceiversInvariants();
Expand All @@ -57,8 +79,18 @@ abstract contract ManagerBase is

bytes32 private constant THRESHOLD_SLOT = bytes32(uint256(keccak256("ntt.threshold")) - 1);

bytes32 private constant UNILATERAL_PAUSE_SLOT =
bytes32(uint256(keccak256("ntt.unilateral_pause")) - 1);

// =============== Storage Getters/Setters ==============================================

function _getUnilateralPauseStorage() internal pure returns (UnilateralPause storage $) {
uint256 slot = uint256(UNILATERAL_PAUSE_SLOT);
assembly ("memory-safe") {
$.slot := slot
}
}

function _getThresholdStorage() private pure returns (_Threshold storage $) {
uint256 slot = uint256(THRESHOLD_SLOT);
assembly ("memory-safe") {
Expand Down Expand Up @@ -273,6 +305,13 @@ abstract contract ManagerBase is
return _getThresholdStorage().num;
}

/// @inheritdoc IManagerBase
function getUnilateralPause() public view returns (UnilateralPause memory) {
UnilateralPause storage u = _getUnilateralPauseStorage();

return UnilateralPause({inbound: u.inbound, outbound: u.outbound});
}

/// @inheritdoc IManagerBase
function isMessageApproved(bytes32 digest) public view returns (bool) {
uint8 threshold = getThreshold();
Expand Down Expand Up @@ -316,6 +355,14 @@ abstract contract ManagerBase is
_unpause();
}

function setInboundPauseStatus(bool status) external onlyOwnerOrPauser {
_getUnilateralPauseStorage().inbound = status;
}

function setOutboundPauseStatus(bool status) external onlyOwnerOrPauser {
_getUnilateralPauseStorage().outbound = status;
}

/// @notice Transfer ownership of the Manager contract and all Transceiver contracts to a new owner.
function transferOwnership(address newOwner) public override onlyOwner {
super.transferOwnership(newOwner);
Expand Down Expand Up @@ -356,7 +403,7 @@ abstract contract ManagerBase is
}

/// @inheritdoc IManagerBase
function removeTransceiver(address transceiver) external onlyOwner {
function removeTransceiver(address transceiver) external onlyOwner outboundWhenPaused {
_removeTransceiver(transceiver);

_Threshold storage _threshold = _getThresholdStorage();
Expand All @@ -372,7 +419,7 @@ abstract contract ManagerBase is
}

/// @inheritdoc IManagerBase
function setThreshold(uint8 threshold) external onlyOwner {
function setThreshold(uint8 threshold) external onlyOwner outboundWhenPaused {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the threshold of the local chain more concerned around receiving inbound messages? For example a deployment may want to change the threshold on one chain only, and they would only have to pause the outbound transfer of that chain, not every other chain that sends to it?

if (threshold == 0) {
revert ZeroThreshold();
}
Expand Down
20 changes: 14 additions & 6 deletions evm/src/NttManager/NttManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ contract NttManager is INttManager, RateLimiter, ManagerBase {
__PausedOwnable_init(msg.sender, msg.sender);
__ReentrancyGuard_init();
_setOutboundLimit(TrimmedAmountLib.max(tokenDecimals()));
_getUnilateralPauseStorage().inbound = true;
_getUnilateralPauseStorage().outbound = true;
}

function _initialize() internal virtual override {
Expand Down Expand Up @@ -104,7 +106,7 @@ contract NttManager is INttManager, RateLimiter, ManagerBase {
bytes32 peerContract,
uint8 decimals,
uint256 inboundLimit
) public onlyOwner {
) public onlyOwner outboundWhenPaused {
if (peerChainId == 0) {
revert InvalidPeerChainIdZero();
}
Expand Down Expand Up @@ -158,7 +160,7 @@ contract NttManager is INttManager, RateLimiter, ManagerBase {
uint256 amount,
uint16 recipientChain,
bytes32 recipient
) external payable nonReentrant whenNotPaused returns (uint64) {
) external payable nonReentrant whenNotPaused outboundNotPaused returns (uint64) {
return
_transferEntryPoint(amount, recipientChain, recipient, recipient, false, new bytes(1));
}
Expand All @@ -171,7 +173,7 @@ contract NttManager is INttManager, RateLimiter, ManagerBase {
bytes32 refundAddress,
bool shouldQueue,
bytes memory transceiverInstructions
) external payable nonReentrant whenNotPaused returns (uint64) {
) external payable nonReentrant whenNotPaused outboundNotPaused returns (uint64) {
return _transferEntryPoint(
amount, recipientChain, recipient, refundAddress, shouldQueue, transceiverInstructions
);
Expand All @@ -182,7 +184,7 @@ contract NttManager is INttManager, RateLimiter, ManagerBase {
uint16 sourceChainId,
bytes32 sourceNttManagerAddress,
TransceiverStructs.NttManagerMessage memory payload
) external onlyTransceiver whenNotPaused {
) external onlyTransceiver whenNotPaused inboundNotPaused {
_verifyPeer(sourceChainId, sourceNttManagerAddress);

// Compute manager message digest and record transceiver attestation.
Expand All @@ -198,7 +200,7 @@ contract NttManager is INttManager, RateLimiter, ManagerBase {
uint16 sourceChainId,
bytes32 sourceNttManagerAddress,
TransceiverStructs.NttManagerMessage memory message
) public whenNotPaused {
) public whenNotPaused inboundNotPaused {
(bytes32 digest, bool alreadyExecuted) =
_isMessageExecuted(sourceChainId, sourceNttManagerAddress, message);

Expand Down Expand Up @@ -241,7 +243,12 @@ contract NttManager is INttManager, RateLimiter, ManagerBase {
}

/// @inheritdoc INttManager
function completeInboundQueuedTransfer(bytes32 digest) external nonReentrant whenNotPaused {
function completeInboundQueuedTransfer(bytes32 digest)
external
nonReentrant
whenNotPaused
inboundNotPaused
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can safely complete here without pausing on inbound transfers? Or do you think it's better to be aggressive on the pausing here?

{
// find the message in the queue
InboundQueuedTransfer memory queuedTransfer = getInboundQueuedTransfer(digest);
if (queuedTransfer.txTimestamp == 0) {
Expand All @@ -266,6 +273,7 @@ contract NttManager is INttManager, RateLimiter, ManagerBase {
payable
nonReentrant
whenNotPaused
outboundNotPaused
returns (uint64)
{
// find the message in the queue
Expand Down
22 changes: 22 additions & 0 deletions evm/src/interfaces/IManagerBase.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ interface IManagerBase {
uint8 num;
}

/// @dev Structure for storing pause information for inbound and outbound transfers
struct UnilateralPause {
bool inbound;
bool outbound;
}

/// @notice Emitted when a message has been attested to.
/// @dev Topic0
/// 0x35a2101eaac94b493e0dfca061f9a7f087913fde8678e7cde0aca9897edba0e5.
Expand Down Expand Up @@ -109,6 +115,18 @@ interface IManagerBase {
/// @param chainId The target chain id
error PeerNotRegistered(uint16 chainId);

/// @notice Error when the receiving message on inbound call when inbound calls are paused
/// @dev Selector 0xab6e758b
error InboundPaused();

/// @notice Error when sending a message when outbound transfers are paused
/// @dev Selector 0xe741d03c
error OutboundPaused();

/// @notice Error when trying to update sensitive values when the outbound is not paused
/// @dev Select 0x37291df0
error NotPausedForUpdate();

/// @notice Fetch the delivery price for a given recipient chain transfer.
/// @param recipientChain The chain ID of the transfer destination.
/// @param transceiverInstructions The transceiver specific instructions for quoting and sending
Expand Down Expand Up @@ -165,6 +183,10 @@ interface IManagerBase {
/// it to be considered valid and acted upon.
function getThreshold() external view returns (uint8);

/// @notice Returns the inbound and outbound pause information for the manager.
/// This is seperate to the general pause feature
function getUnilateralPause() external view returns (UnilateralPause memory);

/// @notice Returns a boolean indicating if the transceiver has attested to the message.
function transceiverAttestedToMessage(
bytes32 digest,
Expand Down
25 changes: 24 additions & 1 deletion evm/test/IntegrationManual.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ contract TestRelayerEndToEndManual is IntegrationHelpers, IRateLimiterEvents {
}

function test_relayerTransceiverAuth() public {
nttManagerChain1.setInboundPauseStatus(false);
nttManagerChain1.setOutboundPauseStatus(false);
nttManagerChain2.setInboundPauseStatus(false);
nttManagerChain2.setOutboundPauseStatus(false);

// Set up sensible WH transceiver peers
_setTransceiverPeers(
[wormholeTransceiverChain1, wormholeTransceiverChain2],
Expand Down Expand Up @@ -143,7 +148,9 @@ contract TestRelayerEndToEndManual is IntegrationHelpers, IRateLimiterEvents {
vm.chainId(chainId2);

// Set bad manager peer (0x1)
nttManagerChain2.setOutboundPauseStatus(true);
nttManagerChain2.setPeer(chainId1, toWormholeFormat(address(0x1)), 9, type(uint64).max);
nttManagerChain2.setOutboundPauseStatus(false);

vm.startPrank(relayer);

Expand All @@ -158,7 +165,9 @@ contract TestRelayerEndToEndManual is IntegrationHelpers, IRateLimiterEvents {
);
vm.stopPrank();

nttManagerChain2.setOutboundPauseStatus(true);
_setManagerPeer(nttManagerChain2, nttManagerChain1, chainId1, 9, type(uint64).max);
nttManagerChain2.setOutboundPauseStatus(false);

// Wrong caller - aka not relayer contract
vm.prank(userD);
Expand Down Expand Up @@ -215,6 +224,20 @@ contract TestRelayerEndToEndManual is IntegrationHelpers, IRateLimiterEvents {
}

function test_relayerWithInvalidWHTransceiver() public {
require(
nttManagerChain1.getUnilateralPause().inbound == true,
"Inbound pause not true by default"
);
require(
nttManagerChain1.getUnilateralPause().outbound == true,
"Outbound pause not true by default"
);

nttManagerChain1.setInboundPauseStatus(false);
nttManagerChain1.setOutboundPauseStatus(false);
nttManagerChain2.setInboundPauseStatus(false);
nttManagerChain2.setOutboundPauseStatus(false);

// Set up dodgy wormhole transceiver peers
wormholeTransceiverChain2.setWormholePeer(chainId1, bytes32(uint256(uint160(address(0x1)))));
wormholeTransceiverChain1.setWormholePeer(
Expand All @@ -239,7 +262,7 @@ contract TestRelayerEndToEndManual is IntegrationHelpers, IRateLimiterEvents {
nttManagerChain1.transfer{
value: wormholeTransceiverChain1.quoteDeliveryPrice(
chainId2, buildTransceiverInstruction(false)
)
)
}(
sendingAmount,
chainId2,
Expand Down
Loading
Loading