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

Invoke contracts' payable method on asset Transfers #108

Closed
belane opened this issue Aug 29, 2019 · 16 comments
Closed

Invoke contracts' payable method on asset Transfers #108

belane opened this issue Aug 29, 2019 · 16 comments

Comments

@belane
Copy link
Member

belane commented Aug 29, 2019

I would love that smart contracts could react to asset transfers, I'm not talking about the Mint function, but rather that a smart contract can react, execute, upon receiving NEO or any asset.
(remember our old friend AplicationR from NEP7?)

After studying several options we found that the solution is right in front of us.
In Native Contracts (NEO/GAS) we can add a call to the address in if the destination is a contract.

In the Transfer method of the native contracts we can include a check of whether the destination account is a contract address, check if it has the payable flag and if so, invoke a specific method of the contract, for example bool Payable()

Payable method can be included in the NEP5 standard and must return True if everything is correct or throw an exception otherwise.

In this way smart contracts can react to payments without users having to build a special transaction, invoke Mint or whatever.
Therefore, Mint invocations won't need a second APPCALL, saving network and blockchain resources.

It also gives to the contract full management of received payments such as denying money receipts, issuing refunds, etc ...

@shargon
Copy link
Member

shargon commented Aug 29, 2019

Who should throw the exception when return false? the CALLER or the CALLED?

@erikzhang
Copy link
Member

This will require all NEP-5 contracts to have dynamic invocation permission.

{
  "contract": "*",
  "methods": ["payable"]
}

Is this worth it?

@igormcoelho
Copy link

igormcoelho commented Aug 30, 2019

Good idea @belane, and good point @erikzhang.

What about if it only applied to Native NEP-5? These can natively support dynamic invoke.

We could have a NEP-Z for Payable (or AlertPayable perhaps), where native NEP-5 is also NEP-Z (AlertPayable), meaning that it will trigger payability condition for other contracts. This way, not all NEP-5 need to be NEP-Z, but if users want to mimic behavior of Native NEP-5, they will be able to.

NEP-Z would only specify the Notification format passed to the invoked contract (on "payable" call), informing that some assets have been sent to it (perhaps in a independent format, that can be reused for NFT, and others too). NEP-Z text could start as simple as: NEP-Z contract agrees to alert (via notification format xxx) and invoke destination contract after an asset transfer. it will be able to respond (and perhaps reject) asset transfer operation.

@erikzhang
Copy link
Member

Agree with @igormcoelho. It shouldn't be included in NEP-5.

@belane
Copy link
Member Author

belane commented Sep 12, 2019

I think it is a worthwhile feature, it brings many possibilities to projects. I only see advantages.
If you agree I can draw a new NEP for this functionality (14?)

The first to implement this functionality should be the native assets, am I right?

btw, why dynamic invocation permission has an extra cost when it's mainly like a call? Can this be adjusted in the new economic model?

@igormcoelho
Copy link

igormcoelho commented Sep 25, 2019

This is a fundamental feature for Neo 3: the future of blockchain. I don't know the format, but we need it.
@erikzhang we discussed today, this is a very important feature. We now think that all contracts having dynamic invoke is not a problem. We just need to discuss a little bit the format, perhaps we find a way that all NEP-5 will do this, without even knowing about it (a transparent solution).

@igormcoelho
Copy link

@erikzhang @shargon I think I found an interesting solution for this. Please correct me if I missed some recent development.

Suppose we don't make contracts pay for dynamic invoke during deployment, but we make users pay (or not) for it, during invocations (a tiny extra cost).

User wallets will likely disable this, unless they need it (as a failed dynamic invoke would cause FAULT if non paid).

Now, before invoking the "payable" method on other contract, we execute some Interop that requires Manifest/MethodList and verify that the remote contract actually requires some payable "reaction".

Ok, now its easy for wallets: if destination requires payable action, it enables dynamic invoke, otherwise it doesn't (and user saves gas).

In any case, this NEP will still be necessary for NEP5 to declare its "willigness" to call remote payable, but I think this resolves the "dynamic invoke dillema".

@erikzhang
Copy link
Member

Maybe we can add a new SYSCALL: Contract.CallIfExists(). Or we can just add a new flag to CallFlags.

@igormcoelho
Copy link

Good idea @erikzhang , automatically checks manifest, simplify a lot coding.
Regarding users paying dynamic invoke fees per execution call, only if needed, instead of contract paying bigger amounts... do you see something like this reasonable? Good thing is that becomes easier to execute parallel tx, if dyn invoke fees are not required in that precise tx.

@erikzhang
Copy link
Member

I'm sorry I think CallIfExists is not a good idea. When you call a contract, you might expect a return value. But if the method does not exist, then there is no return value. Maybe we can add a new SYSCALL Contract.MethodExists() to detect whether the method exists.

@igormcoelho
Copy link

Makes sense Erik, otherwise it would require some advanced C# "out" abstraction, to express meaning on smart contract. MethodExists is good enough.

@shargon
Copy link
Member

shargon commented Oct 28, 2020

I'm sorry I think CallIfExists is not a good idea. When you call a contract, you might expect a return value. But if the method does not exist, then there is no return value. Maybe we can add a new SYSCALL Contract.MethodExists() to detect whether the method exists.

Maybe we can force this syscall if it has a empty return? for this case we need a void

@erikzhang
Copy link
Member

Maybe we can force this syscall if it has a empty return? for this case we need a void

But if this is the case, how do you tell whether the call was successful?

@shargon
Copy link
Member

shargon commented Oct 29, 2020

If it doesn't throw an exception.
Maybe we can create a syscall for this, NEP5.EmitPayment, generate the payment and if it doesn't exists push a value.

@roman-khimov
Copy link
Contributor

I think system-induced invocations don't have any of these problems. onPayment doesn't exist? OK, moving on without invocation. onPayment returns an error? It will always be properly handled. onPayment throws an exception? We can guarantee that it'll be handled by aborting the execution.

@igormcoelho
Copy link

If I understood correctly @shargon , a MethodExists is better because its it never throws exception , always boolean. If the call throws, it could be that it was an error in the method itself (but it exists). So you neeed to catch specific exception, like "methodNorFound", but what happens if your call tries to call other method that doesn't exist... will you know that payable method exists but it failed (so you must fail) or you think method doesn't exist and proceed besides the error (that will cause inconsistency on the contract expecting payable method execution)?
On the other hand, exception approach is good when we dont need to specify parametee lists for MethodExists... if it works, it doesn't throw, naturally.

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