Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow self as default argument #3648

Open
pcaversaccio opened this issue Oct 14, 2023 · 5 comments
Open

allow self as default argument #3648

pcaversaccio opened this issue Oct 14, 2023 · 5 comments

Comments

@pcaversaccio
Copy link
Collaborator

pcaversaccio commented Oct 14, 2023

I think it would be a nice feature to allow non-literal and built-in environment variable values as default arguments.

Example Use Case

# pragma version ^0.3.10


_COLLISION_OFFSET: constant(bytes1) = 0xFF


@external
@pure
def compute_address(salt: bytes32, bytecode_hash: bytes32, deployer: address=self) -> address:
    """
    @dev Returns the address where a contract will be stored if
         deployed via `deployer` using the `CREATE2` opcode.
         Any change in the `bytecode_hash` or `salt` values will
         result in a new destination address.
    @param salt The 32-byte random value used to create the contract
           address.
    @param bytecode_hash The 32-byte bytecode digest of the contract
           creation bytecode.
    @return address The 20-byte address where a contract will be stored.
    """
    data: bytes32 = keccak256(concat(_COLLISION_OFFSET, convert(deployer, bytes20), salt, bytecode_hash))
    return convert(convert(data, uint256) & convert(max_value(uint160), uint256), address)
@pcaversaccio pcaversaccio changed the title Allow non-literal values as default argument Allow non-literal values as default arguments Oct 14, 2023
@pcaversaccio pcaversaccio changed the title Allow non-literal values as default arguments Allow non-literal and built-in environment variable values as default arguments Nov 7, 2023
@charles-cooper
Copy link
Member

i think block.timestamp is allowed. i guess the only question is whether to allow self; i lean against it because it seems like an antipattern. see my notes from offline:

Charles Cooper, [11/7/23 10:18 AM]
i think the question here is if self should be an environment variable

Charles Cooper, [11/7/23 10:18 AM]
And like, it looks more like a storage variable

Charles Cooper, [11/7/23 10:18 AM]
Also its semantics could change depending on execution target

Charles Cooper, [11/7/23 10:19 AM]
Or like it could read from code or storage or something

Charles Cooper, [11/7/23 10:19 AM]
Maybe we should have context.address which compiles to ADDRESS opcode

Charles Cooper, [11/7/23 10:20 AM]
(Which happens here to be the same as self)

Charles Cooper, [11/7/23 10:20 AM]
But it's robust to changes in semantics of self

@pcaversaccio
Copy link
Collaborator Author

pcaversaccio commented Nov 23, 2023

There are a couple of (library) use cases where "function overloading" using self would be helpful. What if we store the self address as an immutable at construction time as part of the code, and use this robust value for such use cases? It's not optimal for bytecode space yes, but would still enable this use case for applications that want to rely on this. This would disallow any semantic changes coming from delegatecalls.

Another approach would be having something like self.address; I once opened an issue here which is loosely connected, but elaborates the issue in the context of interfaces using .address.

@charles-cooper charles-cooper changed the title Allow non-literal and built-in environment variable values as default arguments allow self as default argument Dec 31, 2023
@charles-cooper
Copy link
Member

This would disallow any semantic changes coming from delegatecalls.

delegatecalls change code, so it could indeed have semantic changes!

@pcaversaccio
Copy link
Collaborator Author

Quickly linking to this VIP #3701 which proposes to replace self (currently has type address) with self.address or context.address. This would imply the following code pattern, which I like:

# pragma version ^0.3.10


_COLLISION_OFFSET: constant(bytes1) = 0xFF


@external
@view
def compute_address(salt: bytes32, bytecode_hash: bytes32, deployer: address=self.address) -> address:
    """
    @dev Returns the address where a contract will be stored if
         deployed via `deployer` using the `CREATE2` opcode.
         Any change in the `bytecode_hash` or `salt` values will
         result in a new destination address.
    @param salt The 32-byte random value used to create the contract
           address.
    @param bytecode_hash The 32-byte bytecode digest of the contract
           creation bytecode.
    @return address The 20-byte address where a contract will be stored.
    """
    data: bytes32 = keccak256(concat(_COLLISION_OFFSET, convert(deployer, bytes20), salt, bytecode_hash))
    return convert(convert(data, uint256) & convert(max_value(uint160), uint256), address)

If you think about it, depending on whether you use the default argument or not, the visibility modifier does implicitly change. The overloaded function without the self.address argument in the 4-byte function signature will be view (since reading from the state in the argument) and the "normal" function will be pure (in the context of the above example).

@fubuloubu
Copy link
Member

Typically, instance variables in Python would not be allowed as a default argument, but I think in this case it makes sense to allow self.address, which will only rarely be used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants