Warn on public functions in files that are being imported from

[milancermak]

Consider a Cairo contract made out of two files, main.cairo and library.cairo (shortened for clarity):

# main.cairo

from library import mint_internal, assert_owner

@external
func mint(to : felt, amount : felt):
    assert_owner()
    mint_internal(to, amount)
    return ()
end

# library.cairo

func assert_owner():
    let (caller) = get_caller_address()
    let (owner) = owner_storage.read()
    assert caller = owner
    return ()
end

func mint_internal(to : felt, amount : felt):
    let (balance) = balance_of.read(to)
    balance_of.write(to, balance + amount)
    return ()
end

@external
func test_mint(to, amount):
    mint_internal(to, amount)
    return ()
end

Notice that I only import assert_owner and mint_internal in the main contract. However, when compiled and deployed, the unsecured test_mint function (originally only intended to expose the mint_internal for testing) will also be brough into scope and made available in the deployed contract. This is bad. That's why OpenZeppelin came up with the extensibility pattern.

Can Amarna also warn in these cases, when a module (an "import from" file) contains public functions?

[fcasal]

Hi @milancermak, thanks for suggesting these rules -- they are certainly interesting and would warrant adding them to Amarna.

[fcasal]

Added rule in 1195796ae80781751793ee6782180ff8d409abaa

[milancermak]

Hey @fcasal, if I understand the code correctly, the rule checks only for @external decorator. In my opinion, it would make sense to check for the @l1_handler as well, since it creates a public entrypoint as well. IDK about @view functions - since they are supposed to be read-only (although it's not currently enforced), there's less harm of silently importing them, but I'd say it's not the desired behaviour. Maybe a different rule for @views? Curious to hear your thoughts.

[fcasal]

Thanks for your input on this. I wasn't aware that the same would happen with l1_handler and view. Are there any other cases that are implicitly imported?

[milancermak]

TBH I didn't test it with the l1_handler I'm just assuming it's going to behave the same way because it's a public function / entrypoint. It definitely does happen with view though. I don't know of other cases.