Merge pull request #226 from ava-labs/message-nonce-pre-increment

Pre-increment message nonce

contracts/src/Teleporter/TeleporterMessenger.sol

@@ -49,9 +49,9 @@ contract TeleporterMessenger is ITeleporterMessenger, ReentrancyGuards {
     bytes32 public blockchainID;
 
     // A monotonically incremented integer tracking the total number of messages sent by this TeleporterMessenger contract.
-    // Used to provide uniqueness when generating message IDs for new messages. Initially starts at 1 such that the
-    // nonce value can be used to provide replay protection.
-    uint256 public messageNonce = 1;
+    // Used to provide uniqueness when generating message IDs for new messages. The first message sent will use a
+    // messageNonce of 1 such that the nonce value can be used to provide replay protection for a given message ID.
+    uint256 public messageNonce;
 
     // Tracks the outstanding receipts to send back to a given chain in subsequent messages sent to that chain.
     // The key is the blockchain ID of the other chain, and the value is a queue of pending receipts for messages
@@ -510,7 +510,8 @@ contract TeleporterMessenger is ITeleporterMessenger, ReentrancyGuards {
     function getNextMessageID(bytes32 destinationBlockchainID) external view returns (bytes32) {
         bytes32 blockchainID_ = blockchainID;
         require(blockchainID_ != bytes32(0), "TeleporterMessenger: zero blockchain ID");
-        return calculateMessageID(blockchainID_, destinationBlockchainID, messageNonce);
+        uint256 nextMessageNonce = messageNonce + 1;
+        return calculateMessageID(blockchainID_, destinationBlockchainID, nextMessageNonce);
     }
 
     /**
@@ -596,8 +597,8 @@ contract TeleporterMessenger is ITeleporterMessenger, ReentrancyGuards {
         // If the blockchain ID has yet to be initialized, do so now.
         bytes32 blockchainID_ = initializeBlockchainID();
 
-        // Get the message ID to use for this message.
-        uint256 messageNonce_ = messageNonce;
+        // Get the message ID to use for this message by incrementing it.
+        uint256 messageNonce_ = ++messageNonce;
         bytes32 messageID =
             calculateMessageID(blockchainID_, messageInput.destinationBlockchainID, messageNonce_);
 
@@ -614,9 +615,6 @@ contract TeleporterMessenger is ITeleporterMessenger, ReentrancyGuards {
         });
         bytes memory teleporterMessageBytes = abi.encode(teleporterMessage);
 
-        // Increment the message nonce so the next message will have a different ID
-        ++messageNonce;
-
         // If the fee amount is non-zero, transfer the asset into control of this TeleporterMessenger contract instance.
         // The fee is allowed to be 0 because it's possible for someone to run their own relayer and deliver their own messages,
         // which does not require further incentivization. They still must pay the transaction fee to submit the message, so

contracts/src/Teleporter/tests/AddFeeAmountTests.t.sol

@@ -61,7 +61,7 @@ contract AddFeeAmountTest is TeleporterMessengerTest {
     function testMessageAlreadyDelivered() public {
         // First submit a message with a small fee
         uint256 originalFeeAmount = 10;
-        uint256 expectedNonce = teleporterMessenger.messageNonce();
+        uint256 expectedNonce = _getNextMessageNonce();
         bytes32 messageID =
             _sendTestMessageWithFee(DEFAULT_DESTINATION_BLOCKCHAIN_ID, originalFeeAmount);
 

contracts/src/Teleporter/tests/FallbackReceiveTests.t.sol

@@ -75,7 +75,7 @@ contract FallbackReceiveTest is TeleporterMessengerTest {
 
         // Construct the mock message to be received.
         TeleporterMessage memory messageToReceive = TeleporterMessage({
-            messageNonce: teleporterMessenger.messageNonce(),
+            messageNonce: _getNextMessageNonce(),
             senderAddress: address(this),
             destinationBlockchainID: DEFAULT_DESTINATION_BLOCKCHAIN_ID,
             destinationAddress: address(destinationContract),
@@ -117,7 +117,7 @@ contract FallbackReceiveTest is TeleporterMessengerTest {
 
         // Construct the mock message to be received.
         TeleporterMessage memory messageToReceive = TeleporterMessage({
-            messageNonce: teleporterMessenger.messageNonce(),
+            messageNonce: _getNextMessageNonce(),
             senderAddress: address(this),
             destinationBlockchainID: DEFAULT_DESTINATION_BLOCKCHAIN_ID,
             destinationAddress: address(destinationContract),

contracts/src/Teleporter/tests/GetFeeInfoTests.t.sol

@@ -46,7 +46,7 @@ contract GetFeeInfoTest is TeleporterMessengerTest {
     function testAfterReceipt() public {
         // First submit a message with a small fee
         uint256 feeAmount = 10;
-        uint256 expectedNonce = teleporterMessenger.messageNonce();
+        uint256 expectedNonce = _getNextMessageNonce();
         bytes32 messageID = _sendTestMessageWithFee(DEFAULT_DESTINATION_BLOCKCHAIN_ID, feeAmount);
 
         // Now mock receiving a message back from that subnet with a receipt of the above message.

contracts/src/Teleporter/tests/GetMessageHashTests.t.sol

@@ -22,7 +22,7 @@ contract GetMessageHashTest is TeleporterMessengerTest {
     function testSuccess() public {
         // Submit a message
         TeleporterMessage memory expectedMessage = TeleporterMessage({
-            messageNonce: teleporterMessenger.messageNonce(),
+            messageNonce: _getNextMessageNonce(),
             senderAddress: address(this),
             destinationBlockchainID: DEFAULT_DESTINATION_BLOCKCHAIN_ID,
             destinationAddress: DEFAULT_DESTINATION_ADDRESS,
@@ -47,7 +47,7 @@ contract GetMessageHashTest is TeleporterMessengerTest {
 
     function testMessageAlreadyReceived() public {
         // Submit a message
-        uint256 expectedNonce = teleporterMessenger.messageNonce();
+        uint256 expectedNonce = _getNextMessageNonce();
         bytes32 messageID = _sendTestMessageWithNoFee(DEFAULT_DESTINATION_BLOCKCHAIN_ID);
 
         // Now mock receiving a message back from that subnet with a receipt of the above message.

contracts/src/Teleporter/tests/GetNextMessageIdTests.t.sol

@@ -25,7 +25,7 @@ contract GetNextMessageIDTest is TeleporterMessengerTest {
         bytes32 expectedMessageID = teleporterMessenger.calculateMessageID(
             DEFAULT_DESTINATION_BLOCKCHAIN_ID,
             DEFAULT_DESTINATION_BLOCKCHAIN_ID,
-            teleporterMessenger.messageNonce()
+            _getNextMessageNonce()
         );
 
         // Check the contract reports the same as expected.
@@ -56,7 +56,7 @@ contract GetNextMessageIDTest is TeleporterMessengerTest {
         bytes32 secondExpectedMessageID = teleporterMessenger.calculateMessageID(
             DEFAULT_DESTINATION_BLOCKCHAIN_ID,
             DEFAULT_DESTINATION_BLOCKCHAIN_ID,
-            teleporterMessenger.messageNonce()
+            _getNextMessageNonce()
         );
 
         // Check the contract reports the same as expected, and that is different than the first ID.

contracts/src/Teleporter/tests/GetOutstandingReceiptsToSendTests.t.sol

@@ -54,7 +54,7 @@ contract GetOutstandingReceiptsToSendTest is TeleporterMessengerTest {
         // Now that we have "received" 3 mock messages, when we send a message back to the
         // other chain, we should expect to see the 3 receipts included in the message metadata.
         TeleporterMessage memory expectedMessage =
-            _createMockTeleporterMessage(teleporterMessenger.messageNonce(), hex"deadbeef");
+            _createMockTeleporterMessage(_getNextMessageNonce(), hex"deadbeef");
         expectedMessage.receipts = expectedReceipts;
         expectedMessage.destinationBlockchainID = blockchainID;
         bytes32 expectedMessageID = teleporterMessenger.calculateMessageID(
@@ -95,7 +95,7 @@ contract GetOutstandingReceiptsToSendTest is TeleporterMessengerTest {
 
         // Submit another message to be sent to check that it does not contain any more receipts.
         TeleporterMessage memory nextExpectedMessage =
-            _createMockTeleporterMessage(teleporterMessenger.messageNonce(), hex"deadbeef");
+            _createMockTeleporterMessage(_getNextMessageNonce(), hex"deadbeef");
         nextExpectedMessage.destinationBlockchainID = blockchainID;
         vm.expectCall(
             WARP_PRECOMPILE_ADDRESS,
@@ -166,7 +166,7 @@ contract GetOutstandingReceiptsToSendTest is TeleporterMessengerTest {
         // other chain, we should expect to see the 5 receipts included in the message metadata because
         // that is the max receipt batch size limit.
         TeleporterMessage memory expectedMessage =
-            _createMockTeleporterMessage(teleporterMessenger.messageNonce(), hex"deadbeef");
+            _createMockTeleporterMessage(_getNextMessageNonce(), hex"deadbeef");
         expectedMessage.receipts = expectedReceiptsBatch1;
         expectedMessage.destinationBlockchainID = blockchainID;
         TeleporterFeeInfo memory feeInfo = TeleporterFeeInfo(address(0), 0);
@@ -209,7 +209,7 @@ contract GetOutstandingReceiptsToSendTest is TeleporterMessengerTest {
 
         // Submit another message to be sent to check that it contains the remaining 2 receipts to be sent.
         TeleporterMessage memory nextExpectedMessage =
-            _createMockTeleporterMessage(teleporterMessenger.messageNonce(), hex"deadbeef");
+            _createMockTeleporterMessage(_getNextMessageNonce(), hex"deadbeef");
         nextExpectedMessage.receipts = expectedReceiptsBatch2;
         nextExpectedMessage.destinationBlockchainID = blockchainID;
         vm.expectCall(

contracts/src/Teleporter/tests/MarkReceiptTests.t.sol

@@ -40,7 +40,7 @@ contract MarkReceiptTest is TeleporterMessengerTest {
         uint256[3] memory messageNonces;
         bytes32[3] memory messageIDs;
         for (uint256 i; i < feeRewardInfos.length; ++i) {
-            messageNonces[i] = teleporterMessenger.messageNonce();
+            messageNonces[i] = _getNextMessageNonce();
             messageIDs[i] =
                 _sendTestMessageWithFee(DEFAULT_ORIGIN_BLOCKCHAIN_ID, feeRewardInfos[i].feeAmount);
         }
@@ -109,7 +109,7 @@ contract MarkReceiptTest is TeleporterMessengerTest {
 
     function testReceiptForNoFeeMessage() public {
         // Submit a a mock message with no fee.
-        uint256 sentMessageNonce = teleporterMessenger.messageNonce();
+        uint256 sentMessageNonce = _getNextMessageNonce();
         bytes32 sentMessageID = _sendTestMessageWithNoFee(DEFAULT_ORIGIN_BLOCKCHAIN_ID);
 
         // Mock receiving a message with the a receipts of the mock message sent above.
@@ -153,7 +153,7 @@ contract MarkReceiptTest is TeleporterMessengerTest {
         // Submit a mock message to be sent.
         FeeRewardInfo memory feeRewardInfo =
             FeeRewardInfo(1111111111111111, 0x52A258ED593C793251a89bfd36caE158EE9fC4F8);
-        uint256 sentMessageNonce = teleporterMessenger.messageNonce();
+        uint256 sentMessageNonce = _getNextMessageNonce();
         _sendTestMessageWithFee(DEFAULT_ORIGIN_BLOCKCHAIN_ID, feeRewardInfo.feeAmount);
 
         // Mock receiving a message with the 2 receipts for the  same mock message above.

contracts/src/Teleporter/tests/RedeemRelayerRewardsTests.t.sol

@@ -100,7 +100,7 @@ contract RedeemRelayerRewardsTest is TeleporterMessengerTest {
     // receiving back a message with receipt of that message such that the relayer
     // is able to redeem the reward.
     function _setUpRelayerRewards(FeeRewardInfo memory feeRewardInfo) private {
-        uint256 messageNonce = teleporterMessenger.messageNonce();
+        uint256 messageNonce = _getNextMessageNonce();
         _sendTestMessageWithFee(DEFAULT_ORIGIN_BLOCKCHAIN_ID, feeRewardInfo.feeAmount);
 
         TeleporterMessageReceipt[] memory receipts = new TeleporterMessageReceipt[](1);

contracts/src/Teleporter/tests/RetrySendCrossChainMessageTests.t.sol

@@ -21,7 +21,7 @@ contract RetrySendCrossChainMessageTest is TeleporterMessengerTest {
 
     function testSuccess() public {
         // Send a message
-        uint256 expectedNonce = teleporterMessenger.messageNonce();
+        uint256 expectedNonce = _getNextMessageNonce();
         _sendTestMessageWithFee(DEFAULT_DESTINATION_BLOCKCHAIN_ID, 654456);
         TeleporterMessage memory expectedMessage = TeleporterMessage({
             messageNonce: expectedNonce,
@@ -55,7 +55,7 @@ contract RetrySendCrossChainMessageTest is TeleporterMessengerTest {
 
     function testInvalidMessageHash() public {
         // Send a message, then try to alter it's contents.
-        uint256 expectedNonce = teleporterMessenger.messageNonce();
+        uint256 expectedNonce = _getNextMessageNonce();
         _sendTestMessageWithFee(DEFAULT_DESTINATION_BLOCKCHAIN_ID, 654456);
         TeleporterMessage memory alteredMessage = TeleporterMessage({
             messageNonce: expectedNonce,

contracts/src/Teleporter/tests/SendCrossChainMessageTests.t.sol

@@ -24,7 +24,7 @@ contract SendCrossChainMessageTest is TeleporterMessengerTest {
 
     function testSendMessageNoFee() public {
         // Arrange
-        uint256 expectedMessageNonce = teleporterMessenger.messageNonce();
+        uint256 expectedMessageNonce = _getNextMessageNonce();
         bytes32 expectedMessageID = teleporterMessenger.calculateMessageID(
             DEFAULT_DESTINATION_BLOCKCHAIN_ID,
             DEFAULT_DESTINATION_BLOCKCHAIN_ID,
@@ -73,7 +73,7 @@ contract SendCrossChainMessageTest is TeleporterMessengerTest {
     function testSendMessageWithFee() public {
         // Arrange
         // Construct the message to submit.
-        uint256 expectedMessageNonce = teleporterMessenger.messageNonce();
+        uint256 expectedMessageNonce = _getNextMessageNonce();
         TeleporterMessage memory expectedMessage =
             _createMockTeleporterMessage(expectedMessageNonce, hex"deadbeef");
         TeleporterFeeInfo memory feeInfo =

contracts/src/Teleporter/tests/SendSpecifiedReceiptsTests.t.sol

@@ -45,7 +45,7 @@ contract SendSpecifiedReceiptsTest is TeleporterMessengerTest {
         }
 
         // Mock sending a message back to that chain, which should include the 3 receipts.
-        uint256 expectedMessageNonce = teleporterMessenger.messageNonce();
+        uint256 expectedMessageNonce = _getNextMessageNonce();
         TeleporterMessage memory expectedMessage = TeleporterMessage({
             messageNonce: expectedMessageNonce,
             senderAddress: address(this),
@@ -85,7 +85,7 @@ contract SendSpecifiedReceiptsTest is TeleporterMessengerTest {
             relayerRewardAddress: relayerRewardAddresses[0]
         });
 
-        uint256 newExpectedMessageNonce = teleporterMessenger.messageNonce();
+        uint256 newExpectedMessageNonce = _getNextMessageNonce();
         bytes32 newExpectedMessageID =
             teleporterMessenger.getNextMessageID(DEFAULT_ORIGIN_BLOCKCHAIN_ID);
         TeleporterMessage memory newExpectedMessage = TeleporterMessage({
@@ -158,7 +158,7 @@ contract SendSpecifiedReceiptsTest is TeleporterMessengerTest {
         TeleporterMessageReceipt[] memory expectedReceipts = new TeleporterMessageReceipt[](2);
         expectedReceipts[0] = expectedReceipt;
         expectedReceipts[1] = expectedReceipt;
-        uint256 expectedMessageNonce = teleporterMessenger.messageNonce();
+        uint256 expectedMessageNonce = _getNextMessageNonce();
         bytes32 expectedMessageID =
             teleporterMessenger.getNextMessageID(DEFAULT_DESTINATION_BLOCKCHAIN_ID);
         TeleporterMessage memory expectedMessage = TeleporterMessage({

contracts/src/Teleporter/tests/TeleporterMessengerTest.t.sol

@@ -267,6 +267,10 @@ contract TeleporterMessengerTest is Test {
         });
     }
 
+    function _getNextMessageNonce() internal view returns (uint256) {
+        return teleporterMessenger.messageNonce() + 1;
+    }
+
     function _formatTeleporterErrorMessage(string memory errorMessage)
         internal
         pure