ERCs implementations with custom ABIs can crash

[JeneaVranceanu]

What happened?

This is not to be released within v3. I've added 4.0 tag to this issue and will skip these sentences in the upcoming issues by just adding a 4.0 tag.

The way we have ERCs implemented creates all required conditions for the code to crash if the given ABI is related to a different ERC (e.g. you give ABI of one ERC to another ERC) or ABI is incomplete (e.g. 1 or more functions are missing in the given ABI).

What are the steps to reproduce?

Try initializing ERC712x by passing Web3.Utils.coldWalletABI as the ABI. Calling almost any of the functions in this class will crash because we force unwrap almost each and every createReadOperation and createWriteOperation.

It relates to all ERCs we implement.

What we must do across all ERCs:

1. set public var web3: Web3 as let;
2. remove public var provider: Web3Provider. It's already in web3;
3. set public var address: EthereumAddress as let;
4. set public var abi: String as let;
5. check for other variables in other ERCs besides ERC712x and see which must be marked private, let or completely removed;
6. all functions that call createReadOperation and createWriteOperation must be marked with throws.

Additionally, the following functions must be marked as throws and throw detailed information about what exactly went wrong instead of silently returning nil:

• [ ] ContractProtocol.method
• [ ] ContractProtocol.decodeReturnData
• [ ] ContractProtocol.decodeInputData (there are 2 of them)
• [ ] Web3.Contract.createReadOperation
• [ ] Web3.Contract.createWriteOperation
• [ ] ABI.Element.Constructor.encodeParameters
• [ ] ABI.Element.Constructor.decodeInputData
• [ ] ABI.Element.Function.decodeReturnData
• [ ] ABI.Element.Function.decodeErrorResponse
• [ ] ABI.Element.Function.encodeParameters
• [ ] ABI.Element.Function.decodeInputData
• [ ] ABI.Element.encodeParameters
• [ ] ABI.Element.decodeReturnData
• [ ] ABI.Element.decodeInputData

The list can be potentially extended.

What is the expected behavior?

It must not crash but throw an error instead that an attempt to call an inexistent function took place. The same shall happen in an attempt to encode function parameters.

It won't be much of a problem for developers to add try? to the calls contract.createReadOperation, contract.createWriteOperation, contract.method and function.encodeParameters to get exactly the same behaviour we have now.

What is the error thrown?

What's the stack trace said?

OS version

Library version

https://github.com/web3swift-team/web3swift/tree/5063ca5067700f2826e7a19ae29bedce6b74bcc5

[janndriessen]

Nice one! I'd add a TODO, I think we already had - which is to initialize the contract(which then could be let as well) in the initializer together with the precondition (just how it's done in the lazy computed var). This way the construction of the class fails immediately and the developer knows something is configured wrong.