Unilateral pause implementation #402
Conversation
* Added new structure with dynamic storage slots for the storage of it. * Getter for the struct. * Setter for setting each of the inbound and outbound calls * Added modifiers for functions that cannot be executed while paused and for functions that can ONLY be executed while paused.
There was a problem hiding this comment.
I think generally this makes sense and does what we discussed. Having read the code, unless I'm misunderstanding, I still think this relies quite a lot on documentation for operators on how to coordinate these pausing transactions across chain, do you agree?
If this approach needs documentation and specific instructions for operators too, do you think this sufficiently solves a problem for the added complexity?
| } | ||
|
|
||
| modifier outboundWhenPaused() { | ||
| if (_getUnilateralPauseStorage().outbound == false) { |
There was a problem hiding this comment.
Nit pick but prefer ! vs == false
| external | ||
| nonReentrant | ||
| whenNotPaused | ||
| inboundNotPaused |
There was a problem hiding this comment.
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?
| @@ -372,7 +419,7 @@ abstract contract ManagerBase is | |||
| } | |||
|
|
|||
| /// @inheritdoc IManagerBase | |||
| function setThreshold(uint8 threshold) external onlyOwner { | |||
| function setThreshold(uint8 threshold) external onlyOwner outboundWhenPaused { | |||
There was a problem hiding this comment.
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?
From the Spearbit audit, we noticed that some edits would brick in-flight calls. So, we created a mechanism, alongside proper administration, that should solve this problem.
There is now a way to pause all inbound or all outbound txs. This allows for outbound txs to be paused during a potentially bricking update for in-flight Txs but allow incoming calls. After a certain amount of time that we're confident all Txs have made it, we can change the settings.
Currently, the inboundPause and outboundPause are on all functions where pause is on.
The functions that must be paused in order to update, it's more complicated. I identified that
setPeer,removeTransceiverandsetThresholdall have the capability to break in flights txs. So, these are the only ones that must be outbound paused in order to allow for updates. There is currently no strict time enforcement on this - it's the job of the manager to know this.Open questions