What should the syntax for (i) ABI-formatted logs, and (ii) calls to external contracts be? about vyper

[yograterol]

We could create a reservated variable, by example "Logs" into it variable we can set a dict with every Event for the smart contract, in one place.

Logs = {
    Log1: __log__(arg_1: num),
    Log2: __log__(arg_1: num)
}

And we call:

Logs.Log1(1)

# or

Logs["Log1"](1)

[yograterol]

or external Foo():

[DavidKnott]

I definitely agree with having a contract data type as I think it'll make it easier to think about as opposed to having to call contracts with Item(address) every time.

I'm a little confused about how the scanning would work in practice, as I'm pretty sure that checking if a contract has specific methods will increase gas costs (and we'll have to look into how to check if a contract has a specific method without calling it). At this point I believe forcing a contract to have to already be deployed for it to work seems like more trouble then it's worth (I've been struggle to think of other ways to prevent double recursion).

Maybe it's just because I'm coming from Solidity where abstract contracts are used frequently but it seems to me that by only scanning a contract to get methods takes away some of the code clarity, because although declaring the external functions at the top is more verbose I like the idea of being able to look at the top of a contract and get an idea of all the external calls that are being made.

item: contract {
    foo(bool) -> bool
}
def __init__(_item: contract):
    self.item = _item

def change_item(_item: contract):
    self.item = _item

What do you think of this format, which uses the contract data type but also declares the methods that will be called at the top.

[yograterol]

About of external contracts, I vote to ABI JSON directly.

[0xc1c4da]

Reading the pythonic ABI class definitions is just, well, it's a pleasure. Having that said - JSON ABI is probably preferred because of interoperability with other developer tools. Still thinking about logs

[yograterol]

@jarradh You're right ABI Class is better to read, however JSON ABI as Dictionary could be readable too in Python - Viper.

[fubuloubu]

I was thinking about external contract calls, and let me know what you think about this:

Assumptions

1. contracts are addresses, once initialized on the chain
2. you can get the address of a contract
3. you want to use methods of a contract you are referencing as if you were that contract

It would work like this:

1. There is a new type called contract which is just an address with additional properties
2. When you initialize the contract, you specify the contract address, either explicitly or through an arg
3. Part of the additional properties of a contract type is that you can call any method from that contract with your own arguments, but msg.sender and msg.value are the same
4. Another additional property is that it type-checks during contract initialization to ensure the referenced contract has the methods that are called elsewhere in the contract, and are provided with the right types as args via the ABI (not sure if this is possible, but seems feasible to me). This ensures a user doesn't purposely create a contract that has a method which cannot work due to a method it is calling being specified incorrectly or otherwise not existing.

An example would look something like this:

Contract A (an owned item):

owner: public(address)

def __init__():
    self.owner = msg.sender

def transfer_ownership(new_owner: address):
    assert msg.sender == self.owner
    self.owner = new_owner

Contract B (sale of an owned item):

buyer: address
owner: address
item: contract

@payable
def __init__(_item: contract):
    self.item = _item
    # Implicitly type checks all method calls to Contract A from Contract B 
    # at this point, e.g. ( 
    #      item.get_owner() -> address       [from self.__init__]
    #      item.transfer_ownership(address)  [from self.accept]
    # )
    self.owner = self.item.get_owner()

# Buyer notifies Seller of intent to buy through external means
# (I wish there was like a messaging scheme to directly notify the owner)

# Note: seller should verify what self.item points to before 
# accepting (or rejecting)
def accept():
    assert msg.sender == self.owner
    self.item.transfer_ownership(self.buyer)
    selfdestruct(self.owner)

def reject():
    assert msg.sender == self.owner
    selfdestruct(self.buyer)

Then someone who wants to buy something "owned" can get the contract address for that owned item and provide it to the sale contract as owned item to sell. The point of this example is to demonstrate the process of using this new contract type, which I hope you agree is a simpler, more intuitive way. Let me know what you think, I am not totally familiar with all of the underlying concepts of how this might work, more of a top level view

This might work with inheritance too, perhaps with an inherits type that is like a contract but all the data from the parent contract is actually in the child contract, and the child contract needs to call the initialization method for the parent. After initialization, the child contract is allowed to call any methods of the parent contract and all the effects apply to the data of the child. Additionally, an @override decorator would override any method provided by the parent contract (as well as verify the parent contract has that method to begin with), but you could always re-call that method via self.super.method() (where self.super is the parent contract).

[fubuloubu]

As far as logs, I am not sure why it needs to have it's own log class. Can it be easier to use and just infer the size (in bytes) needed to store the referenced type (assuming it is a base type that is being "printed" to the log)? Then you string them however you like e.g.:

log('MyLog:', 5, sha3("cow"), block.timestamp, "moose")

It seems overkill to me to create a new log type just for a few debug statements.

[DavidKnott]

Contract 1:

def foo(arg1: num) -> num:
    return arg1

Contract 2:

class Foo():
    def foo(arg1: num) -> num: pass

def bar(arg1: address, arg2: num) -> num:
    return Foo(arg1).foo(arg2)

@yograterol @fubuloubu I'm adding in the ability to call other contracts (without raw_call), I currently have the above working. I'd love some feedback on the formatting.

[fubuloubu]

@DavidKnott are these 2 different options? ("Inner code", "outer code") I am totally not following what you are intending here. Maybe a more flushed out example would be clearer.

I believe you are talking about adding inner (worker?) classes in contracts? In my suggestion above, I was thinking that each contract can only be one "class" and you can reference another contract as a class and create an instance of it inside your own contract. That sort of keeps things simpler when discussing contract "inheritance" because each "class" is it's own contract and you can follow the tree of references to understand it by just following the address references. This prevents having too much complexity for the user by allowing inner classes in contract code. Can you give a good use case where I would want an inner class in my contract?

I also have no idea how Foo(), which is defined with no constructor args (as far as I can tell), is called via Foo(arg1) in your "Outer code" example. Am I missing something, or is there a missing part to your example?

[DavidKnott]

@fubuloubu My mistake, I should've been clearer, I'm not talking about inner worker classes. I'm talking about being able to call functions in other contracts without using raw_call. In the example above I'm using class Foo(): to declare the contract and then listing the functions I want to call, in this case the foo function (from inner code).
To reiterate Foo(insert_contract_address_here) creates a contract object that I can call functions that are listed within the above class Foo():
Does that make sense?

[fubuloubu]

So, class is sort of a "prototype" for what methods you're looking to call from a contract that you are going to reference? Does that mean "inner code" is contract 1, and "outer code" is contract 2 in your example?

Further, Foo(arg1: address) creates a copy of contract 1 (referencing using the given address) inside contract 2, and then calls the method .foo(arg2: num) of Foo, which is now operating independantly of contract 1 and performs the method using the given data (e.g. returns arg2)

That's how I'm taking it. I think you need an example where the referenced class has internal (global) data it is making use of, so that we can talk about what data is being operated on in the class.

I'm trying to think of how I would use a class in my "owned item" example above, as that is only showing how referencing might work (even though I mused very briefly on how class inheritance might work)

[DavidKnott]

Yes, exactly class is creating an abstract version of contract 1. But class Foo(): doesn't copy contract 1, it stores the method_id, inputs (with types) and outputs (again with types). This might be a better example:

def test_external_contract_calls():
    contract_1 = """
lucky: num

def __init__(_lucky: num):
    self.lucky = _lucky

def foo() -> num:
    return self.lucky
    """

    lucky_number = 7
    c = get_contract(contract_1, args=[lucky_number])

    contract_2 = """
class Foo():
    def foo() -> num: pass


def bar(arg1: address) -> num:
    return Foo(arg1).foo()
    """
    c2 = get_contract(contract_2)
    
    assert c2.bar(c.address) == lucky_number
    print('Successfully executed an external contract call')

Using your above example if you want to call contract 1 from contract 2 it'd be something like:
Item(insert_address_of_contract_1).transfer_ownership(self.buyer)
Down the road I'd like to add functionality so that you can do this:
self.item = Item(insert_address_of_contract_1)
self.item.transfer_ownership(self.buyer)

[yograterol]

@DavidKnott I like the idea, but It is a bit confuse for me, because if I can use a class just to call an external contract why can't I use a class to define my main contract?

[fubuloubu]

@DavidKnott, okay I think I follow now. I believe my only gripe is that the syntax is a bit awkward, and that you are duplicating some work there (whether that is justified is a different story). My intuition says that if you do a second analysis of types on the usage of that class, you can infer the type signature required without needing to show it explicitly. But if your method makes this feature easier to implement, then I think there's justification for doing it that way, type inferencing doesn't sound fun or scalable.

So, it would be a run-time check that the provided address contains the methods you provided underneath the class declaration, and those class methods are the only ones allowed to be used in the contract's methods. Is `Foo' then a global with type class and the given type signatures for methods? How about this syntax:

item: class {
    foo() -> num, # No in args, returns num
    bar(num), # 1 in arg, no return
    baz(num) -> num # 1 in arg, returns num
}

def __init__(_item: address):
    self.item = _item # run-time check that _item contains 'foo()', 'bar()', and 'baz()'

def foo() -> num:
    return self.item.foo()

[fubuloubu]

Additionally, I believe you would need to instantiate the 1st contract in the 2nd with the lucky number e.g. Foo(arg1: address, arg2: num).foo() -> num otherwise how would Foo() know what lucky was set to? Foo() is an instance of the contract at the specified address, and therefore shall not reference the value set in the original contract. Does that make sense?

[DavidKnott]

Foo() is not an instance but a gateway (used to form function calls) to contract 1. Foo() only needs to know about the functions it's calling which is nice because if you only need to call one function in the Foo() contract you only need to declare one function in it. Does that make sense?

[fubuloubu]

I believe I understand, but that would be talking about referencing not classes. Basically, Foo(address) is telling you that you expect the contract referenced by the given address to contain certain methods, which you are allowed to call below the gateway. But, with your example, the values used are from the original contract (hence we are talking about referencing another contract, instead of reusing it with our own data like how a class typically works IMO) e.g. Foo(address1).foo() == 7

[fubuloubu]

The difference is definitely more semantic, can you use the keyword ref/reference instead of class?

[fubuloubu]

👍 external

[fubuloubu]

I think there should be a rule that these external contract assignmentss have to specified on contract creation (__init__) in order to avoid a scenario where a contract fails during a runtime assignment and therefore the contract is rendered unusable. To phrase differently, the contract needs to fail when trying to run __init__ and not after it has been posted to the chain.

This makes sense actually since this type of parameter cannot have a default value logically

[DavidKnott]

I like external but think that contract assignments should be able allowed anywhere. A common pattern with smart contracts is having contracts that talk back and forth. One way of getting this to work is by deploying contract_1 and then contract_2 and then calling a setup(contract_2: address) function in contract_1. This wouldn't work if contract assignments are only allowed in __init__.

[fubuloubu]

Well, I guess I shouldn't be so limiting. Here is my scenario, if you can make it work then it has my 👍 👍

I am using your method of contract referencing to add a reference to a contract that must contain a foo() method. However, I got the type signature wrong (detected during runtime?) and must re-deploy my contract in order to fix it.

On second thought, this means if you did appropriate testing you could probably avoid this scenario. I guess my worry is that there could be a situation that develops where you don't discover there is a problem until after the contract is in general use, and then that means you'd have to go through a more painful process to redeploy a new contract that people were already using. If it failed at init, then that wouldn't be possible. However, given the overall likelihood of that scenario should be pretty low (if you've done adequete testing), you've convinced me.

[fubuloubu]

Hmm, I guess the other thing is if you haven't initialized the contract reference yet but then go to use a function from it that would be a runtime error that would have to be handled and communicated to the user so that they understand what the problem is. If you always set the reference during contract init (with the option of updating the reference after) then the only runtime error is if the referenced contract does not contain that method, which can be a runtime check when setting the reference so you can avoid never having a scenario where an active contract has a reference that doesn't implement the proper methods.

[fubuloubu]

One more thought, if you don't require a contract reference be set at initialization then it is much easier to create mutual recursion between contracts because you can reference contracts that have been initialized after yours. Somewhat topical after we were talking about Viper's resistance to call stack attacks.

If you make references set on init a requirement, and then allow a reference to be updated after the fact, it is still possible to create mutual recursion but you would have to be much more deliberate to create that mutual recursion because the first contract would need to set it's side of the recursive reference to a bogus contract with the right signature before updating the reference after the 2nd contract is initialized.

Also should put in the assertions for setting a reference that the address cannot be the calling contract itself (avoiding single recursion).

[DavidKnott]

I'll think some more on where / how contract references should be initialized. Definitely a good idea to make sure contracts cannot call themselves (right now my PR doesn't protect against that).

I'm also sticking with class for now, although it's a bit misleading. This is because Viper is currently using ast (a python module written in c) which only parses valid python. I thought of doing something like replace(external, class) before ast is used but that seems a bit hacky.

[fubuloubu]

My original example wayyyy above has syntax that is also valid python and additionally it solves the duplicate work of specifying the desired type signatures manually. However, it requires a method to intrinsically scan the contract code during compilation to determine the proper method type signatures required of the referenced contract and add the proper assertions against that external contract's ABI. A more simplified example using that style:

item: contract

def __init__(_item: contract):
    self.item = _item
    # add run-time assertions that test contract at address _item
    # contains methods 'foo(bool) -> num', ...
    
    # Perhaps this could be an additional constant method 'check_item() -> bool'
    # so other functions can use it with no gas cost (assert check_item())

def change_item(_item: contract):
    self.item = _item # same assertions added as __init__

def bar(flag: bool) -> num:
    # assertions passed on the reference setter, 
    # so this method exists and returns the right type
    return self.item.foo(flag)

Due to the work required to add that additional scanning step, I can see why this might take a little more time to implement, but I think it is ideal as a lot of errors can be detected during compilation, you can decouple the actions of setting the references from using them, and it becomes more obvious that a method failures because the type signature doesn't match what's required.

[fubuloubu]

Definitely much clearer that way, in my opinion.

The only difference between my suggestion (not specifying all the calls) and your proposal (specifying at the top) in practice is that the compiler would do the work of scanning and creating the external call list. You need that list during compilation to decide if the right types are provided to the external call in the code, I think you agree with me there. There is no extra gas cost for that scanning, it's only during the compilation stage to ensure types match up. Explicitly writing them at the top is a clear summary of what you're doing in the code below and also makes it easier for the compiler; but the cost is that the contract writer has to remember to keep that in sync with how they are using it as they're writing. Given the paradigm make it easier for the reader than the writer, I'm leaning towards your way now for clarity's sake.

Now, the second part of my suggestion is performing that type-checking again during the assignment of the reference. This would be on chain, so there would be increased gas costs here. The point of this check would be to ensure you couldn't set a reference to a contract that couldn't fulfill the desired external calls elsewhere in the contract, basically changing the location of the failure from when the external call is made to when the reference is assigned so it is clear that the reference assignment cannot fulfill something else in the contract. That clarifies run-time errors IMO (assignment and external calls are in separate contexts). Additionally, this would prevent an attack where you had some reference set, and everything was working fine, and then the attacker came along and set the reference to some contract that doesn't have the right calls, preventing others from withdrawing funds or whatever while they had their way with it.

Last, ensuring a contract must already be deployed to assign the first reference comes from mandating that the reference is set in __init__(). This is optional, but then you'd have unset references which would make contract calls fail until the reference is set anyways.

[DavidKnott]

Glad we agree on the first part.

For the second part of things I see the merits of moving the error up to instantiation but am not so sure about it in practice. I think it needs to be evaluated further and the decision should be made depending how much it increases gas costs and how "seeing" another contracts function works (without calling it).

For the __init__ functionality I'm leaning against it for now at least for the first implementation. Though I'd like to discuss it again after we have something to play around with and test for vulnerabilities.

[fubuloubu]

Further thoughts. Having this contract type should allow more complex interactions such as lists and maps, as behind the hood the contract type should basically be an address type with extra properties. I was thinking of some use cases, which I'll summarize in the following example:

friends: contract {
    receive_message(bytes <= 192)
}[num]
num_friends: num
messages: (bytes <= 192)[num]
num_messages: num

def add_friend(_friend: contract):
    self.friends[self.num_friends] = _friend
    self.num_friends += 1

def send_message(friend_num: num, message: bytes <= 192):
    self.friends[friend_num].receive_message(message)

def receive_message(_message: bytes <= 192):
    self.messages[self.num_messages] = _message
    self.num_messages += 1

In this example, two (or more) instances of this contract can send each other messages. I could see trying to build out the add-friend functionality to build a secure messaging platform with key signing, etc. but for now I hope you see the point here. I don't think anything about what we've discussed precludes these extensions, although tests will be needed to tell this works of course.

What do you think @DavidKnott?

[DavidKnott]

@fubuloubu I think this is a great idea! If I understand it correctly is seems like it'll add functionality (the ability to make calls to multiple contracts with the same methods with no cost).

[fubuloubu]

Not sure I follow about the additional functionality (or how it would be free lol). I was thinking of it more like allowing these contract addresses to be stored in lists/maps and then allowing calls from contracts in those lists/maps in the same way a single contract might be used.

Imagine the contract in my example is deployed twice for two different users.

[DavidKnott]

Closing this, now the external contract calling and logging have been implemented

[fubuloubu]

Was a hell of a wall of text lol