mini-rollup of several issues #498
Conversation
… falling through caused erroneous decoding.
As it was it would throw a decode error instead of returning nil this was exposed when I fixed the coding key for callOnBlock, which typically isn't present
changed 'Signle' to 'Single' corrected all calling occurences
| @@ -117,7 +117,7 @@ extension ABIDecoder { | |||
| var subpointer: UInt64 = 32; | |||
| var toReturn = [AnyObject]() | |||
| for _ in 0 ..< length { | |||
| let (v, c) = decodeSignleType(type: subType, data: elementItself, pointer: subpointer) | |||
| let (v, c) = decodeSingleType(type: subType, data: elementItself, pointer: subpointer) | |||
There was a problem hiding this comment.
Is it just me, or these lines are equal?
There was a problem hiding this comment.
spelling correction from gn to ng in Single. it's hard to spot
| @@ -52,13 +52,13 @@ extension TransactionOptions: Decodable { | |||
|
|
|||
| public init(from decoder: Decoder) throws { | |||
| let container = try decoder.container(keyedBy: CodingKeys.self) | |||
| if let gasLimit = try decodeHexToBigUInt(container, key: .gas) { | |||
| if let gasLimit = try decodeHexToBigUInt(container, key: .gas, allowOptional:true) { | |||
There was a problem hiding this comment.
While working with Web3+structures source i'm kind of confused:
As i see this lib took it's struct from web3.js, where some struct fields are optional. But at the same time i've found Eth JSON-RPC specification Transaction scheme where none of that are optional. It looks kind official tho. I've lack of experience in Ethereum itself, so maybe you're could explain to me why there's such ignorance of specification happening even in kind of it's official implementation like web3.js?
There was a problem hiding this comment.
the problem here is that to code expects to drop down to provide a default if the call to decodeHexToBigUInt returns with nil. decodeHexToBigUInt will throw Web3Error.dataError instead of return nil unless allowOptional is set to true. So this if let is meaningless without the allowOptional: true.
On a larger scope, yes nonce should never be optional, I was not trying to address that issue. That should be handled at a higher level. The input here could come from a number of sources, where nonce may no be known yet. The same goes for the various gas values, as if this is a 1559 transaction those values may not exist. Furthermore, if the transaction is pending and not mined yet, those values may also not be present, as they are not known yet. Issue #469
| self.gasLimit = .manual(gasLimit) | ||
| } else { | ||
| self.gasLimit = .automatic | ||
| } | ||
|
|
||
| if let gasPrice = try decodeHexToBigUInt(container, key: .gasPrice) { | ||
| if let gasPrice = try decodeHexToBigUInt(container, key: .gasPrice, allowOptional:true) { |
There was a problem hiding this comment.
Here's i see that optionality happening since this struct should work with both EIP-1559 Transactions and legacy transaction where the first one don't have that field already, am i correct?
There was a problem hiding this comment.
same as my previous comment
I've consolidated a number of smaller issues into one here, for ease
fallthroughwith the expected result-- closes ABIDecoder.decodeSignleType() [sic] fails to decode dynamic arrays in structs #429
-- this exposed a bug in the TransactionOptions decoding
-- as it was it would throw an invalid value error, instead of falling to the provided default in the
if let-- consequently this probably fixes Invalid value from Ethereum node #469