Skip to content

Commit

Permalink
fix: check invalid address starknet
Browse files Browse the repository at this point in the history
  • Loading branch information
@obatirou
obatirou committed Sep 18, 2024
1 parent aba7414 commit 7d50fe6
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 12 deletions.
25 changes: 25 additions & 0 deletions solidity_contracts/src/CairoPrecompiles/DualVmToken.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ contract DualVmToken {
CAIRO SPECIFIC VARIABLES
//////////////////////////////////////////////////////////////*/

/// @dev The prime number used in the Starknet field
uint256 public constant STARKNET_FIELD_PRIME = 2 ** 251 + 17 * 2 ** 192 + 1;

/// @dev The address of the starknet token to call
uint256 public immutable starknetToken;

Expand All @@ -38,6 +41,13 @@ contract DualVmToken {
/// @dev Emitted when the allowance of a starknet address spender over the owner's tokens is set
event ApprovalStarknet(address indexed owner, uint256 indexed spender, uint256 amount);

/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/

/// @dev Emitted when an invalid starknet address is used
error InvalidStarknetAddress();

/*//////////////////////////////////////////////////////////////
METADATA ACCESS
//////////////////////////////////////////////////////////////*/
Expand Down Expand Up @@ -82,6 +92,9 @@ contract DualVmToken {
}

function _balanceOf(uint256 starknetAddress) private view returns (uint256) {
if (starknetAddress >= STARKNET_FIELD_PRIME) {
revert InvalidStarknetAddress();
}
uint256[] memory balanceOfCallData = new uint256[](1);
balanceOfCallData[0] = starknetAddress;
bytes memory returnData = starknetToken.staticcallCairo("balance_of", balanceOfCallData);
Expand Down Expand Up @@ -138,6 +151,9 @@ contract DualVmToken {
}

function _allowance(uint256 owner, uint256 spender) private view returns (uint256) {
if (owner >= STARKNET_FIELD_PRIME || spender >= STARKNET_FIELD_PRIME) {
revert InvalidStarknetAddress();
}
uint256[] memory allowanceCallData = new uint256[](2);
allowanceCallData[0] = owner;
allowanceCallData[1] = spender;
Expand Down Expand Up @@ -184,6 +200,9 @@ contract DualVmToken {
}

function _approve(uint256 spender, uint256 amount) private {
if (spender >= STARKNET_FIELD_PRIME) {
revert InvalidStarknetAddress();
}
// Split amount in [low, high]
uint128 amountLow = uint128(amount);
uint128 amountHigh = uint128(amount >> 128);
Expand Down Expand Up @@ -217,6 +236,9 @@ contract DualVmToken {
}

function _transfer(uint256 to, uint256 amount) private {
if (to >= STARKNET_FIELD_PRIME) {
revert InvalidStarknetAddress();
}
// Split amount in [low, high]
uint128 amountLow = uint128(amount);
uint128 amountHigh = uint128(amount >> 128);
Expand Down Expand Up @@ -294,6 +316,9 @@ contract DualVmToken {
}

function _transferFrom(uint256 from, uint256 to, uint256 amount) private {
if (from >= STARKNET_FIELD_PRIME || to >= STARKNET_FIELD_PRIME) {
revert InvalidStarknetAddress();
}
// Split amount in [low, high]
uint128 amountLow = uint128(amount);
uint128 amountHigh = uint128(amount >> 128);
Expand Down
93 changes: 82 additions & 11 deletions tests/end_to_end/CairoPrecompiles/test_dual_vm_token.py
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import pytest
import pytest_asyncio
from eth_utils import keccak

from kakarot_scripts.utils.kakarot import deploy as deploy_kakarot
from kakarot_scripts.utils.starknet import deploy as deploy_starknet
Expand Down Expand Up @@ -86,6 +87,13 @@ async def test_should_return_balance_of_starknet_address(
)
assert balance_owner_starknet == balance_owner_evm

async def test_should_revert_balance_of_invalid_address(
self, starknet_token, dual_vm_token, owner
):
evm_error = keccak("InvalidStarknetAddress()".encode())[:4]
with cairo_error(evm_error):
await dual_vm_token.balanceOfStarknetAddress(2**256 - 1)

class TestActions:
async def test_should_transfer(
self, starknet_token, dual_vm_token, owner, other
Expand Down Expand Up @@ -117,7 +125,7 @@ async def test_should_revert_transfert_insufficient_balance(
owner.address, 1, caller_eoa=other.starknet_contract
)

async def test_should_transfer_to_starknet_address(
async def test_should_transfer_starknet_address(
self, starknet_token, dual_vm_token, owner, other
):
amount = 1
Expand All @@ -142,6 +150,13 @@ async def test_should_transfer_to_starknet_address(
assert balance_owner_before - amount == balance_owner_after
assert balance_other_before + amount == balance_other_after

async def test_should_revert_transfer_starknet_address_invalid_address(
self, starknet_token, dual_vm_token, owner
):
evm_error = keccak("InvalidStarknetAddress()".encode())[:4]
with cairo_error(evm_error):
await dual_vm_token.transferStarknetAddress(2**256 - 1, 1)

async def test_should_revert_transfer_to_starknet_address_insufficient_balance(
self, starknet_token, dual_vm_token, owner, other
):
Expand Down Expand Up @@ -174,7 +189,7 @@ async def test_should_approve(
)
assert allowance_after == allowance_before + amount

async def test_should_approve_spender_starknet_address(
async def test_should_approve_starknet_address(
self, starknet_token, dual_vm_token, owner, other
):
amount = 1
Expand All @@ -199,6 +214,13 @@ async def test_should_approve_spender_starknet_address(
)
assert allowance_after == allowance_before + amount

async def test_should_revert_approve_starknet_address_invalid_address(
self, starknet_token, dual_vm_token, owner
):
evm_error = keccak("InvalidStarknetAddress()".encode())[:4]
with cairo_error(evm_error):
await dual_vm_token.approveStarknetAddress(2**256 - 1, 1)

async def test_allowance_owner_starknet_address(
self, starknet_token, dual_vm_token, other
):
Expand All @@ -221,6 +243,26 @@ async def test_allowance_owner_starknet_address(
)
assert allowance_after == allowance_before + amount

async def test_should_revert_allowance_starknet_address_owner_invalid_address(
self, starknet_token, dual_vm_token, owner, other
):
with cairo_error(
"EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles"
):
await dual_vm_token.allowanceStarknetAddressOwner(
2**256 - 1, other.address
)

async def test_should_revert_allowance_starknet_address_spender_invalid_address(
self, starknet_token, dual_vm_token, owner
):
with cairo_error(
"EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles"
):
await dual_vm_token.allowanceStarknetAddressSpender(
owner.address, 2**256 - 1
)

async def test_allowance_owner_and_spender_starknet_address(
self, starknet_token, dual_vm_token
):
Expand Down Expand Up @@ -301,7 +343,7 @@ async def test_should_revert_transfer_from_insufficient_allowance(
other.address, owner.address, 1, caller_eoa=owner.starknet_contract
)

async def test_should_transfer_from_starknet_address(
async def test_should_transfer_from_starknet_address_from(
self, starknet_token, dual_vm_token, owner, other
):
amount = 1
Expand Down Expand Up @@ -332,7 +374,17 @@ async def test_should_transfer_from_starknet_address(
assert balance_owner_before - amount == balance_owner_after
assert balance_other_before + amount == balance_other_after

async def test_should_revert_transfer_from_starknet_address_insufficient_balance(
async def test_should_revert_transfer_from_starnet_address_from_invalid_address(
self, starknet_token, dual_vm_token, owner, other
):
with cairo_error(
"EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles"
):
await dual_vm_token.transferFromStarknetAddressFrom(
2**256 - 1, other.address, 1, caller_eoa=other.starknet_contract
)

async def test_should_revert_transfer_from_starknet_address_from_insufficient_balance(
self, starknet_token, dual_vm_token, owner, other
):
# No wrapping of errors for OZ 0.10 contracts
Expand All @@ -346,7 +398,7 @@ async def test_should_revert_transfer_from_starknet_address_insufficient_balance
caller_eoa=owner.starknet_contract,
)

async def test_should_revert_transfer_from_starknet_address_insufficient_allowance(
async def test_should_revert_transfer_from_starknet_address_from_insufficient_allowance(
self, starknet_token, dual_vm_token, owner, other
):
# No wrapping of errors for OZ 0.10 contracts
Expand All @@ -358,7 +410,7 @@ async def test_should_revert_transfer_from_starknet_address_insufficient_allowan
caller_eoa=other.starknet_contract,
)

async def test_should_transfer_from_to_starknet_address(
async def test_should_transfer_from_starknet_address_to(
self, starknet_token, dual_vm_token, owner, other
):
amount = 1
Expand Down Expand Up @@ -389,7 +441,7 @@ async def test_should_transfer_from_to_starknet_address(
assert balance_owner_before - amount == balance_owner_after
assert balance_other_before + amount == balance_other_after

async def test_should_revert_transfer_from_to_starknet_address_insufficient_balance(
async def test_should_revert_transfer_from_starknet_address_to_insufficient_balance(
self, starknet_token, dual_vm_token, owner, other
):
# No wrapping of errors for OZ 0.10 contracts
Expand All @@ -403,7 +455,7 @@ async def test_should_revert_transfer_from_to_starknet_address_insufficient_bala
caller_eoa=owner.starknet_contract,
)

async def test_should_revert_transfer_from_to_starknet_address_insufficient_allowance(
async def test_should_revert_transfer_from_starknet_address_to_insufficient_allowance(
self, starknet_token, dual_vm_token, owner, other
):
# No wrapping of errors for OZ 0.10 contracts
Expand All @@ -415,7 +467,17 @@ async def test_should_revert_transfer_from_to_starknet_address_insufficient_allo
caller_eoa=other.starknet_contract,
)

async def test_should_transfer_from_and_to_starknet_address(
async def test_should_revert_transfer_from_starknet_address_to_invalid_address(
self, starknet_token, dual_vm_token, owner, other
):
with cairo_error(
"EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles"
):
await dual_vm_token.transferFromStarknetAddressTo(
owner.address, 2**256 - 1, 1, caller_eoa=other.starknet_contract
)

async def test_should_transfer_from_starknet_address_from_and_to(
self, starknet_token, dual_vm_token, owner, other
):
amount = 1
Expand Down Expand Up @@ -446,7 +508,7 @@ async def test_should_transfer_from_and_to_starknet_address(
assert balance_owner_before - amount == balance_owner_after
assert balance_other_before + amount == balance_other_after

async def test_should_revert_transfer_from_and_to_starknet_address_insufficient_balance(
async def test_should_revert_transfer_from_starknet_address_from_and_to_insufficient_balance(
self, starknet_token, dual_vm_token, owner, other
):
# No wrapping of errors for OZ 0.10 contracts
Expand All @@ -460,7 +522,7 @@ async def test_should_revert_transfer_from_and_to_starknet_address_insufficient_
caller_eoa=owner.starknet_contract,
)

async def test_should_revert_transfer_from_and_to_starknet_address_insufficient_allowance(
async def test_should_revert_transfer_from_starknet_address_from_and_to_insufficient_allowance(
self, starknet_token, dual_vm_token, owner, other
):
# No wrapping of errors for OZ 0.10 contracts
Expand All @@ -472,6 +534,15 @@ async def test_should_revert_transfer_from_and_to_starknet_address_insufficient_
caller_eoa=other.starknet_contract,
)

async def test_should_revert_transfer_from_starknet_address_from_and_to_invalid_address(
self, starknet_token, dual_vm_token, owner, other
):
evm_error = keccak("InvalidStarknetAddress()".encode())[:4]
with cairo_error(evm_error):
await dual_vm_token.transferFromStarknetAddressFromAndTo(
2**256 - 1, 2**256 - 1, 1, caller_eoa=other.starknet_contract
)

async def test_should_revert_tx_cairo_precompiles(
self, starknet_token, dual_vm_token, owner, other
):
Expand Down
2 changes: 1 addition & 1 deletion tests/utils/errors.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,6 @@ def cairo_error(message=None):
else:
error = re.search(r"Error message: (.*)", str(e.value))
error = error.group(1) if error else str(e.value)
assert message in error, f"Expected {message}, got {error}"
assert str(message) in error, f"Expected {message}, got {error}"
finally:
pass

0 comments on commit 7d50fe6

Please sign in to comment.