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.

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

evm: Remove redundant threshold setting in script #578

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

patitonar
Copy link

This PR removes an explicit call to INttManager.setThreshold(1) that is not needed given that the threshold was already set to 1 in a previous call to IManagerBase(nttManager).setTransceiver(transceiver); when the first transceiver was added.

function setTransceiver(
address transceiver
) external onlyOwner {
_setTransceiver(transceiver);
_Threshold storage _threshold = _getThresholdStorage();
// We do not automatically increase the threshold here.
// Automatically increasing the threshold can result in a scenario
// where in-flight messages can't be redeemed.
// For example: Assume there is 1 Transceiver and the threshold is 1.
// If we were to add a new Transceiver, the threshold would increase to 2.
// However, all messages that are either in-flight or that are sent on
// a source chain that does not yet have 2 Transceivers will only have been
// sent from a single transceiver, so they would never be able to get
// redeemed.
// Instead, we leave it up to the owner to manually update the threshold
// after some period of time, ideally once all chains have the new Transceiver
// and transfers that were sent via the old configuration are all complete.
// However if the threshold is 0 (the initial case) we do increment to 1.
if (_threshold.num == 0) {
_threshold.num = 1;
}
emit TransceiverAdded(transceiver, _getNumTransceiversStorage().enabled, _threshold.num);
_checkThresholdInvariants();
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant