From 14cb89832f38cce4f1762146e007333b8a1ee11d Mon Sep 17 00:00:00 2001 From: Gustavo Grieco <31542053+ggrieco-tob@users.noreply.github.com> Date: Fri, 23 Sep 2022 15:13:36 +0200 Subject: [PATCH 1/6] Added L2 to L1 message failure as a possible issue --- .../consider_L2_to_L1_message_failure.md | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 not-so-smart-contracts/cairo/consider_L2_to_L1_message_failure.md diff --git a/not-so-smart-contracts/cairo/consider_L2_to_L1_message_failure.md b/not-so-smart-contracts/cairo/consider_L2_to_L1_message_failure.md new file mode 100644 index 00000000..4e7780af --- /dev/null +++ b/not-so-smart-contracts/cairo/consider_L2_to_L1_message_failure.md @@ -0,0 +1,29 @@ +# Consider L2 to L1 message failure + +In Starknet, [Ethereum contracts can send messages from L1 to L2, using a bridge](https://www.cairo-lang.org/docs/hello_starknet/l1l2.html#messages-from-l1-to-l2). However, it is not guaranteed that the message will be processed by the sequencer. +For instance, a message can fail to be processed if there is a sudden spike in the gas price and the value provided is too low. For that reason, Starknet developers provided a +[API to cancel on-going messages](https://docs.starknet.io/docs/L1-L2%20Communication/messaging-mechanism/#l1--l2-messages) + +# Example + +Suppose that the following code to initiate L2 deposits from L1, taking the tokens from the user + +```solidity +IERC20 public constant token; //some token to deposit on L2 + +function depositToL2(uint256 to, uint256 amount) public returns (bool) { + require(token.transferFrom(to, address(this), amount)); + .. + StarknetCore.sendMessageToL2(..); + return true; +} +``` + +If the L2 message is never processed by the sequencer, users will never receive their tokens either in L1 or L2, so they need to way to cancel it. + +As a more real example, a recent [AAVE audit](https://github.com/aave-starknet-project/aave-starknet-bridge/pull/106#issue-1336925381) highlighed this issue and required to add code to cancel messages. + +# Mitigations + +When sending a message from L1 to L2, consider the case where the message is never processed by the sequencer. This can block either the contract to reach certain state or users to retreive their funds. +Consider using `startL1ToL2MessageCancellation` and `cancelL1ToL2Message` to cancel ongoing messages. From 57b362d839a3fcf7a9646821204651b5a8f73f11 Mon Sep 17 00:00:00 2001 From: Gustavo Grieco <31542053+ggrieco-tob@users.noreply.github.com> Date: Fri, 23 Sep 2022 15:52:15 +0200 Subject: [PATCH 2/6] Title was wrong --- ..._message_failure.md => consider_L1_to_L2_message_failure.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename not-so-smart-contracts/cairo/{consider_L2_to_L1_message_failure.md => consider_L1_to_L2_message_failure.md} (97%) diff --git a/not-so-smart-contracts/cairo/consider_L2_to_L1_message_failure.md b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md similarity index 97% rename from not-so-smart-contracts/cairo/consider_L2_to_L1_message_failure.md rename to not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md index 4e7780af..31680afd 100644 --- a/not-so-smart-contracts/cairo/consider_L2_to_L1_message_failure.md +++ b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md @@ -1,4 +1,4 @@ -# Consider L2 to L1 message failure +# Consider L1 to L2 message failure In Starknet, [Ethereum contracts can send messages from L1 to L2, using a bridge](https://www.cairo-lang.org/docs/hello_starknet/l1l2.html#messages-from-l1-to-l2). However, it is not guaranteed that the message will be processed by the sequencer. For instance, a message can fail to be processed if there is a sudden spike in the gas price and the value provided is too low. For that reason, Starknet developers provided a From 9e19a4170a2b90f79bf7500ba6cb8255fbaaf330 Mon Sep 17 00:00:00 2001 From: Gustavo Grieco <31542053+ggrieco-tob@users.noreply.github.com> Date: Fri, 23 Sep 2022 16:51:49 +0200 Subject: [PATCH 3/6] Update consider_L1_to_L2_message_failure.md --- .../cairo/consider_L1_to_L2_message_failure.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md index 31680afd..0a88e263 100644 --- a/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md +++ b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md @@ -1,6 +1,6 @@ # Consider L1 to L2 message failure -In Starknet, [Ethereum contracts can send messages from L1 to L2, using a bridge](https://www.cairo-lang.org/docs/hello_starknet/l1l2.html#messages-from-l1-to-l2). However, it is not guaranteed that the message will be processed by the sequencer. +In Starknet, [Ethereum contracts can send messages from L1 to L2, using a bridge](https://starknet.io/docs/hello_starknet/l1l2.html#messages-from-l1-to-l2). However, it is not guaranteed that the message will be processed by the sequencer. For instance, a message can fail to be processed if there is a sudden spike in the gas price and the value provided is too low. For that reason, Starknet developers provided a [API to cancel on-going messages](https://docs.starknet.io/docs/L1-L2%20Communication/messaging-mechanism/#l1--l2-messages) From 8b8cc7248018329d80c85d3be104219ae58eda67 Mon Sep 17 00:00:00 2001 From: Gustavo Grieco <31542053+ggrieco-tob@users.noreply.github.com> Date: Fri, 23 Sep 2022 17:21:27 +0200 Subject: [PATCH 4/6] Fixed --- .../cairo/consider_L1_to_L2_message_failure.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md index 0a88e263..7ceb8490 100644 --- a/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md +++ b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md @@ -6,7 +6,7 @@ For instance, a message can fail to be processed if there is a sudden spike in t # Example -Suppose that the following code to initiate L2 deposits from L1, taking the tokens from the user +Suppose that the following code to initiate L2 deposits from L1, taking the tokens from the user: ```solidity IERC20 public constant token; //some token to deposit on L2 @@ -19,11 +19,10 @@ function depositToL2(uint256 to, uint256 amount) public returns (bool) { } ``` -If the L2 message is never processed by the sequencer, users will never receive their tokens either in L1 or L2, so they need to way to cancel it. +If a L1 message is never processed by the sequencer, users will never receive their tokens either in L1 or L2, so they need to way to cancel it. As a more real example, a recent [AAVE audit](https://github.com/aave-starknet-project/aave-starknet-bridge/pull/106#issue-1336925381) highlighed this issue and required to add code to cancel messages. # Mitigations -When sending a message from L1 to L2, consider the case where the message is never processed by the sequencer. This can block either the contract to reach certain state or users to retreive their funds. -Consider using `startL1ToL2MessageCancellation` and `cancelL1ToL2Message` to cancel ongoing messages. +When sending a message from L1 to L2, consider the case where a message is never processed by the sequencer. This can block either the contract to reach certain state or users to retreive their funds. Allow to use `startL1ToL2MessageCancellation` and `cancelL1ToL2Message` to cancel ongoing messages, if needed. From 32e449457870275e71fc5dd5600c24421adea857 Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Fri, 28 Oct 2022 14:34:48 +0200 Subject: [PATCH 5/6] Update not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md Co-authored-by: Simone <79767264+smonicas@users.noreply.github.com> --- .../cairo/consider_L1_to_L2_message_failure.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md index 7ceb8490..875ed537 100644 --- a/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md +++ b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md @@ -25,4 +25,4 @@ As a more real example, a recent [AAVE audit](https://github.com/aave-starknet-p # Mitigations -When sending a message from L1 to L2, consider the case where a message is never processed by the sequencer. This can block either the contract to reach certain state or users to retreive their funds. Allow to use `startL1ToL2MessageCancellation` and `cancelL1ToL2Message` to cancel ongoing messages, if needed. +When sending a message from L1 to L2, consider the case where a message is never processed by the sequencer. This can block either the contract to reach certain state or users to retrieve their funds. Allow to use `startL1ToL2MessageCancellation` and `cancelL1ToL2Message` to cancel ongoing messages, if needed. From 2de6923be94dd6545fbfed063bac805507d186ed Mon Sep 17 00:00:00 2001 From: Feist Josselin Date: Fri, 28 Oct 2022 14:34:56 +0200 Subject: [PATCH 6/6] Update not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md Co-authored-by: Simone <79767264+smonicas@users.noreply.github.com> --- .../cairo/consider_L1_to_L2_message_failure.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md index 875ed537..013f033a 100644 --- a/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md +++ b/not-so-smart-contracts/cairo/consider_L1_to_L2_message_failure.md @@ -19,7 +19,7 @@ function depositToL2(uint256 to, uint256 amount) public returns (bool) { } ``` -If a L1 message is never processed by the sequencer, users will never receive their tokens either in L1 or L2, so they need to way to cancel it. +If a L1 message is never processed by the sequencer, users will never receive their tokens either in L1 or L2, so they need a way to cancel it. As a more real example, a recent [AAVE audit](https://github.com/aave-starknet-project/aave-starknet-bridge/pull/106#issue-1336925381) highlighed this issue and required to add code to cancel messages.