Does Neo need a mechanism to validate that a contract hasn't been updated before invocation?

[devhawk]

Background:

• 2775

• 2675

@Liaojinghui has opened two different issues to address the tension between the ability to update contracts and the need for contract verification. Instead of proposing a specific solution, I think we should discuss this tension directly and decide if it needs to be addressed at all. If we decide the tension needs to be addressed, we can then discuss specific remedies either here in this issue or open a new issue.

I don't want to put words in @Liaojinghui 's mouth, but I will take a shot at describing this tension. Please correct me @Liaojinghui if I get this wrong.

Before taking a dependency on a deployed contract, developers often ensure the contract being invoked does what it claims to do by verifying the contract's implementation. OneGate Explorer includes a mechanism for publishing contract source code to enable this type of verification. However, the ability to update contracts without changing the invocation hash leads to a scenario where the contract being invoked is no longer the contract that was verified.

[devhawk]

I think we do need to address this. I can see how this capability could lead to a scenario where a bad actor tricks other contract developers into invoking a malicious contract:

• Bad actor deploys useful contract + publishes source code on OneGate
• Other developers take dependency Bad Actor's contract
• Bad actor updates their contract to do something malicious
• Other developer's contracts invoke malicious contract without realizing it has been updated

[shargon]

@devhawk that's could happens in ethereum if you use proxys

[erikzhang]

public void CallWithVersion(UInt160 hash, ushort version, string method, CallFlags callFlags, params object[] args)
{
    Contract contract = ContractManagement.GetContract(hash);
    if (contract?.UpdateCounter != version) throw new Exception();
    Contract.Call(hash, method, callFlags, args);
}

[roman-khimov]

ContractManagement.GetContract(hash);

It doubles the cost of a call, basically. But maybe that's the way to go, it can be provided by the contract framework/SDK right now without any modifications to the core. Then if it's popular enough the cost could be reduced by creating System.Contract.CallWithVersion specifically for it.

At the same time I doubt it'll solve all of the problems. For example, NEP-17 or NEP-11 contracts using onNEPXXPayment can't limit versions of called contracts. Then exchanges also can have different opinions on whether they should continue to work with a token after contract update or stop doing that until it's administratively enabled again.

[devhawk]

@devhawk that's could happens in ethereum if you use proxys

When I first learned that we were making this change in N3, I was against it. However, your point here is the reason I changed my mind. Without the ability to update a contract, there's strong incentive for developers to proxy every contract. This increases their workload AND takes away any opportunity for the platform to manage the process.

[devhawk]

It doubles the cost of a call, basically. But maybe that's the way to go, it can be provided by the contract framework/SDK right now without any modifications to the core. Then if it's popular enough the cost could be reduced by creating System.Contract.CallWithVersion specifically for it.

I definitely think we should reduce the cost of this path - either by adding CallWithVersion to the platform or maybe by adding a cheap GetContractUpdateCounter operation.

However, it's doubling the cost of the call operation - there's still the GAS cost of whatever the contract being called will do. Do we have a good sense as to how much a typical cross contract invocation costs? if the call operation is a small percentage of the overall cost, then doubling it isn't really a big deal. In my opinion, the larger the typical percentage of the overall invocation cost, the more important it is for us to address it in the platform rather than leaving developers to build their own solutions.

At the same time I doubt it'll solve all of the problems. For example, NEP-17 or NEP-11 contracts using onNEPXXPayment can't limit versions of called contracts. Then exchanges also can have different opinions on whether they should continue to work with a token after contract update or stop doing that until it's administratively enabled again.

I don't think token contracts + on payment calls have the same potential security concerns. I mean, if there's a way to exploit an on payment call, can't a bad actor just implement it directly rather than starting with a useful implementation and then upgrading to a malicious version?

[shargon]

What about add the expected version in the permissions manifest?

[devhawk]

What about add the expected version in the permissions manifest?

Seems more complicated to implement, but it's an approach worth considering