add felt comparison/l1 to l2 conversion

Author: technovision99

not-so-smart-contracts/cairo/L1_to_L2_address_conversion/README.md

@@ -0,0 +1,30 @@
+# L1 to L2 Address Conversion
+
+In Starknet, addresses are of type `felt` while on L1 addresses are of type `uint160`. Thus, in order to pass around address types during cross layer messaging, the address variable is typically given as a `uint256`. However, this can create an issue where an address on L1 can map to the zero address (or an unexpected address) on L2. This is because the primitive type in Cairo is the `felt`, which is in between the range ` 0 < x < P`, where P is the prime order of the curve. Typically we have `P = 2^251 + 17 * 2^192 + 1`.
+
+# Example
+
+Suppose that the following code to initiate L2 deposits from L1. The first example has no checks on the `to` parameter and thus depending on the users' address, it is possible to transfer tokens to an unexpected address on L2. The second example, however, verifies to make sure this check cannot happen.
+
+```solidity 
+uint256 public constant STARKNET_FIELD_PRIME; // the prime order P of the elliptic curve used 
+IERC20 public constant token; //some token to deposit on L2
+event Deposited(uint256 sender, uint256 amount);
+
+function badDepositToL2(uint256 to,  uint256 amount) public returns (bool) {
+    token.transferFrom(to, address(this),amount);
+    emit Deposited(to,amount); // this message gets processed on L2
+    return true;
+}
+
+function goodDepositToL2(uint256 to,  uint256 amount) public returns (bool) {
+    require(to !=0 && to < STARKNET_FIELD_PRIME, "invalid address"); //verifies 0 < to < P 
+    token.transferFrom(to, address(this),amount);
+    emit Deposited(to,amount); // this message gets processed on L2
+    return true;
+}
+```
+
+# Mitigations
+
+When sending a message from L1 to L2, remember to verify parameters, especially user supplied params. Remember that the type and range of Cairo's default `felt` type is less than the `uint256` type used by Solidity. 

not-so-smart-contracts/cairo/incorrect_felt_comparison/README.md

@@ -0,0 +1,40 @@
+# Incorrect Felt Comparison
+
+In cairo, there are two methods for comparison, in particular for the less than or equal to operator we have the methods `assert_le` and `assert_nn_le`. `assert_le` asserts that a number a is less than or equal to b, regardless of the size of a, while `assert_nn_le` will also assert that a is non-negative, ie not greater than the `RANGE_CHECK_BOUND` value of `2^128`: https://github.com/starkware-libs/cairo-lang/blob/master/src/starkware/cairo/common/math.cairo#L66-L67
+
+# Example
+
+Suppose that a codebase uses the following checks regarding a hypothetical ERC20 token. In the first function, it may be possible that `value` is in fact greater than `max_supply`, yet because the function does not verify `value <0` the assertion will incorrectly pass. The second function, however, asserts that `0 < value < max_supply`, which will correctly not let an incorrect `value` go through the assertion.
+
+```cairo
+@storage_var
+func max_supply() -> (res: felt):
+end
+
+@external
+func bad_comparison{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}():
+    let (value: felt) = ERC20.total_supply()
+    assert_le{range_check_ptr=range_check_ptr}(value, max_supply.read())
+
+   # do something...
+
+    return ()
+end
+
+@external
+func better_comparison{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}():
+    let (value: felt) = ERC20.total_supply()
+    assert_nn_le{range_check_ptr=range_check_ptr}(value, max_supply.read())
+
+   # do something...
+
+    return ()
+
+    
+end
+```
+
+
+
+# Mitigations
+Review all felt comparisons closely. Determine what sort of behavior the comparison should have, and if `assert_nn_le` is more appropriate.