-
Notifications
You must be signed in to change notification settings - Fork 487
Description
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:
- set
public var web3: Web3aslet; - remove
public var provider: Web3Provider. It's already inweb3; - set
public var address: EthereumAddressaslet; - set
public var abi: Stringaslet; - check for other variables in other
ERCs besidesERC712xand see which must be markedprivate,letor completely removed; - all functions that call
createReadOperationandcreateWriteOperationmust be marked withthrows.
Additionally, the following functions must be marked as throws and throw detailed information about what exactly went wrong instead of silently returning nil:
ContractProtocol.methodContractProtocol.decodeReturnDataContractProtocol.decodeInputData(there are 2 of them)Web3.Contract.createReadOperationWeb3.Contract.createWriteOperationABI.Element.Constructor.encodeParametersABI.Element.Constructor.decodeInputDataABI.Element.Function.decodeReturnDataABI.Element.Function.decodeErrorResponseABI.Element.Function.encodeParametersABI.Element.Function.decodeInputDataABI.Element.encodeParametersABI.Element.decodeReturnDataABI.Element.decodeInputDataTo pick up a draggable item, press the space bar. While dragging, use the arrow keys to move the item. Press space again to drop the item in its new position, or press escape to cancel.
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

Activity
janndriessen commentedon Jan 15, 2023
Nice one! I'd add a
TODO, I think we already had - which is to initialize thecontract(which then could beletas 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.