Skip to content

Commit

Permalink
Only allow retrying failed transmissions
Browse files Browse the repository at this point in the history
  • Loading branch information
DeividasK committed Aug 5, 2024
1 parent 4953204 commit c19c0fb
Show file tree
Hide file tree
Showing 4 changed files with 153 additions and 9 deletions.
112 changes: 112 additions & 0 deletions contracts/.gas-snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_NoConfigurationContract() (gas: 154832)
CapabilitiesRegistry_AddCapabilitiesTest:test_AddCapability_WithConfiguration() (gas: 178813)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_CalledByNonAdmin() (gas: 24723)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_CapabilityExists() (gas: 145703)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_ConfigurationContractDoesNotMatchInterface() (gas: 94606)
CapabilitiesRegistry_AddCapabilitiesTest:test_RevertWhen_ConfigurationContractNotDeployed() (gas: 92961)
CapabilitiesRegistry_AddDONTest:test_AddDON() (gas: 372302)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_CalledByNonAdmin() (gas: 19273)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 169752)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 239789)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DuplicateCapabilityAdded() (gas: 249596)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 116890)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_FaultToleranceIsZero() (gas: 43358)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_NodeAlreadyBelongsToWorkflowDON() (gas: 343924)
CapabilitiesRegistry_AddDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (gas: 180150)
CapabilitiesRegistry_AddNodeOperatorsTest:test_AddNodeOperators() (gas: 184135)
CapabilitiesRegistry_AddNodeOperatorsTest:test_RevertWhen_CalledByNonAdmin() (gas: 17602)
CapabilitiesRegistry_AddNodeOperatorsTest:test_RevertWhen_NodeOperatorAdminAddressZero() (gas: 18498)
CapabilitiesRegistry_AddNodesTest:test_AddsNodeParams() (gas: 358448)
CapabilitiesRegistry_AddNodesTest:test_OwnerCanAddNodes() (gas: 358414)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingDuplicateP2PId() (gas: 301229)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingNodeWithInvalidCapability() (gas: 55174)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingNodeWithInvalidNodeOperator() (gas: 24895)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_AddingNodeWithoutCapabilities() (gas: 27669)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 25108)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 27408)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_SignerAddressEmpty() (gas: 27047)
CapabilitiesRegistry_AddNodesTest:test_RevertWhen_SignerAddressNotUnique() (gas: 309679)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_DeprecatesCapability() (gas: 89807)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_EmitsEvent() (gas: 89935)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_RevertWhen_CalledByNonAdmin() (gas: 22944)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 16231)
CapabilitiesRegistry_DeprecateCapabilitiesTest:test_RevertWhen_CapabilityIsDeprecated() (gas: 91264)
CapabilitiesRegistry_GetCapabilitiesTest:test_ReturnsCapabilities() (gas: 135553)
CapabilitiesRegistry_GetDONsTest:test_CorrectlyFetchesDONs() (gas: 65468)
CapabilitiesRegistry_GetDONsTest:test_DoesNotIncludeRemovedDONs() (gas: 64924)
CapabilitiesRegistry_GetHashedCapabilityTest:test_CorrectlyGeneratesHashedCapabilityId() (gas: 11428)
CapabilitiesRegistry_GetHashedCapabilityTest:test_DoesNotCauseIncorrectClashes() (gas: 13087)
CapabilitiesRegistry_GetNodeOperatorsTest:test_CorrectlyFetchesNodeOperators() (gas: 36407)
CapabilitiesRegistry_GetNodeOperatorsTest:test_DoesNotIncludeRemovedNodeOperators() (gas: 38692)
CapabilitiesRegistry_GetNodesTest:test_CorrectlyFetchesNodes() (gas: 65288)
CapabilitiesRegistry_GetNodesTest:test_DoesNotIncludeRemovedNodes() (gas: 73533)
CapabilitiesRegistry_RemoveDONsTest:test_RemovesDON() (gas: 54761)
CapabilitiesRegistry_RemoveDONsTest:test_RevertWhen_CalledByNonAdmin() (gas: 15647)
CapabilitiesRegistry_RemoveDONsTest:test_RevertWhen_DONDoesNotExist() (gas: 16550)
CapabilitiesRegistry_RemoveNodeOperatorsTest:test_RemovesNodeOperator() (gas: 36122)
CapabilitiesRegistry_RemoveNodeOperatorsTest:test_RevertWhen_CalledByNonOwner() (gas: 15816)
CapabilitiesRegistry_RemoveNodesTest:test_CanAddNodeWithSameSignerAddressAfterRemoving() (gas: 115151)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenDONDeleted() (gas: 287716)
CapabilitiesRegistry_RemoveNodesTest:test_CanRemoveWhenNodeNoLongerPartOfDON() (gas: 561023)
CapabilitiesRegistry_RemoveNodesTest:test_OwnerCanRemoveNodes() (gas: 73376)
CapabilitiesRegistry_RemoveNodesTest:test_RemovesNode() (gas: 75211)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 25053)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_NodeDoesNotExist() (gas: 18418)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_NodePartOfCapabilitiesDON() (gas: 385369)
CapabilitiesRegistry_RemoveNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 18408)
CapabilitiesRegistry_TypeAndVersionTest:test_TypeAndVersion() (gas: 9796)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_CalledByNonAdmin() (gas: 19415)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_CapabilityDoesNotExist() (gas: 152914)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DONDoesNotExist() (gas: 17835)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DeprecatedCapabilityAdded() (gas: 222996)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateCapabilityAdded() (gas: 232804)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_DuplicateNodeAdded() (gas: 107643)
CapabilitiesRegistry_UpdateDONTest:test_RevertWhen_NodeDoesNotSupportCapability() (gas: 163357)
CapabilitiesRegistry_UpdateDONTest:test_UpdatesDON() (gas: 371909)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_CalledByNonAdminAndNonOwner() (gas: 20728)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorAdminIsZeroAddress() (gas: 20052)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorDoesNotExist() (gas: 19790)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_RevertWhen_NodeOperatorIdAndParamLengthsMismatch() (gas: 15430)
CapabilitiesRegistry_UpdateNodeOperatorTest:test_UpdatesNodeOperator() (gas: 37034)
CapabilitiesRegistry_UpdateNodesTest:test_CanUpdateParamsIfNodeSignerAddressNoLongerUsed() (gas: 256371)
CapabilitiesRegistry_UpdateNodesTest:test_OwnerCanUpdateNodes() (gas: 162166)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_AddingNodeWithInvalidCapability() (gas: 35873)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_CalledByAnotherNodeOperatorAdmin() (gas: 29200)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_CalledByNonNodeOperatorAdminAndNonOwner() (gas: 29377)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeDoesNotExist() (gas: 29199)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_NodeSignerAlreadyAssignedToAnotherNode() (gas: 31326)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_P2PIDEmpty() (gas: 29165)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByCapabilityDON() (gas: 470910)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_RemovingCapabilityRequiredByWorkflowDON() (gas: 341191)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_SignerAddressEmpty() (gas: 29058)
CapabilitiesRegistry_UpdateNodesTest:test_RevertWhen_UpdatingNodeWithoutCapabilities() (gas: 27587)
CapabilitiesRegistry_UpdateNodesTest:test_UpdatesNodeParams() (gas: 162220)
KeystoneForwarder_ReportTest:test_Report_ConfigVersion() (gas: 2002057)
KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverInterfaceNotSupported() (gas: 128934)
KeystoneForwarder_ReportTest:test_Report_FailedDeliveryWhenReceiverNotContract() (gas: 130621)
KeystoneForwarder_ReportTest:test_Report_SuccessfulDelivery() (gas: 359123)
KeystoneForwarder_ReportTest:test_Report_SuccessfulRetryWithMoreGas() (gas: 423982)
KeystoneForwarder_ReportTest:test_RevertWhen_AnySignatureIsInvalid() (gas: 86348)
KeystoneForwarder_ReportTest:test_RevertWhen_AnySignerIsInvalid() (gas: 118486)
KeystoneForwarder_ReportTest:test_RevertWhen_ReportHasDuplicateSignatures() (gas: 94516)
KeystoneForwarder_ReportTest:test_RevertWhen_ReportHasIncorrectDON() (gas: 75930)
KeystoneForwarder_ReportTest:test_RevertWhen_ReportHasInexistentConfigVersion() (gas: 76320)
KeystoneForwarder_ReportTest:test_RevertWhen_ReportIsMalformed() (gas: 45585)
KeystoneForwarder_ReportTest:test_RevertWhen_RetryingInvalidContractTransmission() (gas: 143354)
KeystoneForwarder_ReportTest:test_RevertWhen_RetryingSuccessfulTransmission() (gas: 353272)
KeystoneForwarder_ReportTest:test_RevertWhen_TooFewSignatures() (gas: 55292)
KeystoneForwarder_ReportTest:test_RevertWhen_TooManySignatures() (gas: 56050)
KeystoneForwarder_SetConfigTest:test_RevertWhen_ExcessSigners() (gas: 20184)
KeystoneForwarder_SetConfigTest:test_RevertWhen_FaultToleranceIsZero() (gas: 88057)
KeystoneForwarder_SetConfigTest:test_RevertWhen_InsufficientSigners() (gas: 14533)
KeystoneForwarder_SetConfigTest:test_RevertWhen_NotOwner() (gas: 88766)
KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingDuplicateSigners() (gas: 114570)
KeystoneForwarder_SetConfigTest:test_RevertWhen_ProvidingZeroAddressSigner() (gas: 114225)
KeystoneForwarder_SetConfigTest:test_SetConfig_FirstTime() (gas: 1540541)
KeystoneForwarder_SetConfigTest:test_SetConfig_WhenSignersAreRemoved() (gas: 1535211)
KeystoneForwarder_TypeAndVersionTest:test_TypeAndVersion() (gas: 9641)
KeystoneRouter_SetConfigTest:test_AddForwarder_RevertWhen_NotOwner() (gas: 10978)
KeystoneRouter_SetConfigTest:test_RemoveForwarder_RevertWhen_NotOwner() (gas: 10923)
KeystoneRouter_SetConfigTest:test_RemoveForwarder_Success() (gas: 17599)
KeystoneRouter_SetConfigTest:test_Route_RevertWhen_UnauthorizedForwarder() (gas: 18552)
KeystoneRouter_SetConfigTest:test_Route_Success() (gas: 76407)
11 changes: 7 additions & 4 deletions contracts/src/v0.8/keystone/KeystoneForwarder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,15 @@ contract KeystoneForwarder is OwnerIsCreator, ITypeAndVersion, IRouter {
if (!s_forwarders[msg.sender]) revert UnauthorizedForwarder();

TransmissionInfo memory transmission = s_transmissions[transmissionId];
uint256 gasLeft = gasleft();
if (transmission.transmitter != address(0) && transmission.gasLimit >= gasLeft)
revert AlreadyAttempted(transmissionId);
if (transmission.success || transmission.invalidReceiver) revert AlreadyAttempted(transmissionId);

s_transmissions[transmissionId].transmitter = transmitter;
s_transmissions[transmissionId].gasLimit = uint88(gasLeft);
s_transmissions[transmissionId].gasLimit = uint80(gasleft());

if (receiver.code.length == 0) {
s_transmissions[transmissionId].invalidReceiver = true;
return false;
}

bool success;
bytes memory payload = abi.encodeCall(IReceiver.onReport, (metadata, validatedReport));
Expand Down
19 changes: 18 additions & 1 deletion contracts/src/v0.8/keystone/interfaces/IRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,18 +15,35 @@ interface IRouter {
enum TransmissionState {
NOT_ATTEMPTED,
SUCCEEDED,
INVALID_RECEIVER,
FAILED
}

struct TransmissionInfo {
address transmitter;
// This is true if the receiver is not a contract or does not implement the
// `IReceiver` interface.
bool invalidReceiver;
// Whether the transmission attempt was successful. If `false`, the
// transmission can be retried with an increased gas limit.
bool success;
// The amount of gas allocated for the `IReceiver.onReport` call. uint88
// allows storing gas for known EVM block gas limits.
// Ensures that the minimum gas requested by the user is available during
// the transmission attempt. If the transmission fails (indicated by a
// `false` success state), it can be retried with an increased gas limit.
uint88 gasLimit;
uint80 gasLimit;
}

struct Transmission {
address transmitter;
TransmissionState state;
// The amount of gas allocated for the `IReceiver.onReport` call. uint88
// allows storing gas for known EVM block gas limits.
// Ensures that the minimum gas requested by the user is available during
// the transmission attempt. If the transmission fails (indicated by a
// `false` success state), it can be retried with an increased gas limit.
uint80 gasLimit;
}

function addForwarder(address forwarder) external;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,24 @@ contract KeystoneForwarder_ReportTest is BaseTest {
s_forwarder.report(address(s_receiver), report, reportContext, signatures);
}

function test_RevertWhen_AlreadyAttempted() public {
s_forwarder.report(address(s_receiver), report, reportContext, signatures);
function test_RevertWhen_RetryingSuccessfulTransmission() public {
s_forwarder.report{gas: 400_000}(address(s_receiver), report, reportContext, signatures);

bytes32 transmissionId = s_forwarder.getTransmissionId(address(s_receiver), executionId, reportId);
vm.expectRevert(abi.encodeWithSelector(IRouter.AlreadyAttempted.selector, transmissionId));
s_forwarder.report(address(s_receiver), report, reportContext, signatures);
// Retyring with more gas
s_forwarder.report{gas: 450_000}(address(s_receiver), report, reportContext, signatures);
}

function test_RevertWhen_RetryingInvalidContractTransmission() public {
// Receiver is not a contract
address receiver = address(404);
s_forwarder.report{gas: 400_000}(receiver, report, reportContext, signatures);

bytes32 transmissionId = s_forwarder.getTransmissionId(receiver, executionId, reportId);
vm.expectRevert(abi.encodeWithSelector(IRouter.AlreadyAttempted.selector, transmissionId));
// Retyring with more gas
s_forwarder.report{gas: 450_000}(receiver, report, reportContext, signatures);
}

function test_Report_SuccessfulDelivery() public {
Expand Down Expand Up @@ -178,7 +190,7 @@ contract KeystoneForwarder_ReportTest is BaseTest {

// TODO: Add error for insufficient gas

function test_Report_SuccessfulRetryWithGas() public {
function test_Report_SuccessfulRetryWithMoreGas() public {
s_forwarder.report{gas: 150_000}(address(s_receiver), report, reportContext, signatures);

// Expect to fail with the receiver running out of gas
Expand Down

0 comments on commit c19c0fb

Please sign in to comment.