fix: replaced AnyObject with Any #725

JeneaVranceanu · 2023-01-13T15:34:55Z

Summary of Changes

Dropping the use of AnyObject in favour of Any.
This change was performed by simply replacing as AnyObject and as [AnyObject] with empty string ``. One particular case where AnyObject was still required is the `EIP712Hashable` in the implementation of a `func hash()`. Due to the use of reflection, all values are bridged to Objective-C representation (that is a guess) and to check a value for nullability we have to convert `Any` to `AnyObject` and check if it `is NSNull`.

WARNING: All tests pass except the ones related to ENS. Ignore that, I'll fix and post an update on this PR when I find the reason for these failures.

Test Data or Screenshots

By submitting this pull request, you are confirming the following:

I have reviewed the Contribution Guidelines.
I have performed a self-review of my own code.
I have updated my repository to match the develop branch.
I have included test data or screenshots that prove my fix is effective or that my feature works.
I have checked that all tests work and swiftlint is not throwing any errors/warnings.

…ith Any

JeneaVranceanu · 2023-01-13T15:37:49Z

The workflow has been deliberately cancelled as tests will surely fail.

Sources/web3swift/Utils/EIP/EIP712.swift

JeneaVranceanu · 2023-01-13T15:40:28Z

Sources/web3swift/Utils/EIP/EIP712.swift

            case let hashable as EIP712Hashable:
                result = try hashable.hash()
            default:
+                /// Cast to `AnyObject` is required. Otherwise, `nil` value will fail this condition.
                if (field as AnyObject) is NSNull {


This is the only case found so far where AnyObject is required.
field value can be nil even though the type it has is Any (interesting edge case).
So checking for field == nil will return false if field is actually nil.

yaroslavyaroslav · 2023-01-13T16:07:30Z

Sources/Web3Core/Contract/ContractProtocol.swift

@@ -121,7 +121,7 @@ public protocol ContractProtocol {
    /// - Returns: Encoded data for a given parameters, which is should be assigned to ``CodableTransaction.data`` property
    func deploy(bytecode: Data,
                constructor: ABI.Element.Constructor?,
-                parameters: [AnyObject]?,
+                parameters: [Any]?,


I see that in some places you drop a type erasure totally, while in others you've left it, but change the target type to Any? Within my review I assume that you do this in following logic: in case if type erasure it was removed, in case its required target type has ben set to Any. Tell me if i'm wrong with that, resolve this otherwise.

Could you please provide one or more examples where I did the following:

in some places you drop a type erasure totally

I'll start to self-review this PR only now.

To remove any misunderstanding: the goal I had in mind is to eliminate the use AnyObject as much as possible. I simply performed the replacement of AnyObject with Any, ran the tests, fixed almost all issues that occurred (except ENS tests) and opened this PR.

AnyObject uses bridged types and basically changes the type of the object when it can.
For example:

String -> NSString;

Int -> NSNumber;

Array -> NSArray;

struct -> __SwiftValue;

etc.

While using Any we preserve this information.

And this is where it gets important!
When we have Int8, Int16, Int.. and UInt[8-64] these all get represented as NSNumber if we cast them to AnyObject. And here is an example of what will happen:

We do not want to represent Int8 as Int or the other way around.

I guess I've finally got the point. AnyObject is about bridging swift types to an Obj-C one, but with type erasure. Guess it's some internal apple demand at its most.

Sources/Web3Core/Transaction/Envelope/EIP1559Envelope.swift

Sources/web3swift/Tokens/ERC20/ERC20BaseProperties.swift

Sources/web3swift/Utils/EIP/EIP712.swift

…ing protocol to classes)

# Conflicts: # Sources/Web3Core/RLP/RLP.swift # Sources/Web3Core/Transaction/Envelope/EIP1559Envelope.swift # Sources/Web3Core/Transaction/Envelope/EIP2930Envelope.swift # Sources/Web3Core/Transaction/Envelope/LegacyEnvelope.swift # Sources/web3swift/Tokens/ERC1155/Web3+ERC1155.swift # Sources/web3swift/Tokens/ERC1376/Web3+ERC1376.swift # Sources/web3swift/Tokens/ERC1400/Web3+ERC1400.swift # Sources/web3swift/Tokens/ERC1410/Web3+ERC1410.swift # Sources/web3swift/Tokens/ERC1594/Web3+ERC1594.swift # Sources/web3swift/Tokens/ERC1633/Web3+ERC1633.swift # Sources/web3swift/Tokens/ERC1643/Web3+ERC1643.swift # Sources/web3swift/Tokens/ERC1644/Web3+ERC1644.swift # Sources/web3swift/Tokens/ERC20/Web3+ERC20.swift # Sources/web3swift/Tokens/ERC721/Web3+ERC721.swift # Sources/web3swift/Tokens/ERC721x/Web3+ERC721x.swift # Sources/web3swift/Tokens/ERC777/Web3+ERC777.swift # Sources/web3swift/Tokens/ERC888/Web3+ERC888.swift # Sources/web3swift/Tokens/ST20/Web3+ST20.swift # Sources/web3swift/Tokens/ST20/Web3+SecurityToken.swift # Sources/web3swift/Utils/ENS/ENSBaseRegistrar.swift # Sources/web3swift/Utils/ENS/ENSRegistry.swift # Sources/web3swift/Utils/ENS/ENSReverseRegistrar.swift # Sources/web3swift/Utils/ENS/ETHRegistrarController.swift

…innet instead of 0

JeneaVranceanu · 2023-01-14T11:53:12Z

Sources/Web3Core/RLP/RLP.swift

@@ -112,14 +112,13 @@ public struct RLP {
        return encoded.bytes[0]
    }

-    // FIXME: Make encode generic to avoid casting it's argument to [AnyObject]


Removed this comment as I do not see a flexible enough solution for us to implement proper generics when we operate on an array that will hold not just one generic type but multiple different types at once.
This is where Any really shines so to speak.

I see the type Any as the last resort but if we are careful with it everything should go smoothly.

JeneaVranceanu · 2023-01-14T11:54:31Z

Sources/Web3Core/Structure/Event+Protocol.swift

@@ -77,7 +77,7 @@ public enum Networks {

    static let allValues = [Mainnet, Ropsten, Kovan, Rinkeby]

-    public static func fromInt(_ networkID: UInt) -> Networks? {
+    public static func fromInt(_ networkID: UInt) -> Networks {


We never really returned nil from here or at least we do not return any nils now.

JeneaVranceanu · 2023-01-14T16:03:22Z

Sources/web3swift/Utils/EIP/EIP681.swift

            var inputNumber: Int = 0
            var inputs = [ABI.Element.InOut]()
            for comp in queryItems {
                if let inputType = try? ABITypeParser.parseTypeString(comp.name) {
                    guard let rawValue = comp.value,
                          let functionArgument = await parseFunctionArgument(inputType,
                                                                             rawValue.trimmingCharacters(in: .whitespacesAndNewlines),
-                                                                             chainID: code.chainID ?? 0,
+                                                                             chainID: code.chainID,


A bug was introduced here by setting default value to 0.
Now, if the chainID is nil the value of Networks.Mainnnet.chainID is used in the parseFunctionArgument function.
Note, that the issue is related only to parsing ENS addresses, and to be specifically the host part, e.g. ethereum:someaddress.eth@8834. If we do not have the chainID, that's the 8834 value in the example we pick 0 as default. Then verification of the someaddress.eth fails because of that chain ID value.

I do not think this is the final correct solution.
Instead what must be thought through is the following: what is the default behaviour when chain ID is not provided? Is the Mainnet chain ID a correct default value?

I'll look into this once again to figure out if we need to change this part again.

…NotNil

JeneaVranceanu · 2023-01-14T16:18:56Z

@yaroslavyaroslav @janndriessen I'm done with this PR. Except the issue mentioned in this comment: #725 (comment)

I'll push the fix for that (if there will be a need for any fixes) in a different PR to make it easier to review later.

yaroslavyaroslav · 2023-01-14T18:06:00Z

I guess I'm unable to review this pr because of ci/cd failure. @JeneaVranceanu

It fixes the issue when trying to call func element() with [Any].

JeneaVranceanu · 2023-01-14T22:27:08Z

I guess I'm unable to review this pr because of ci/cd failure. @JeneaVranceanu

Haha, seem like it was too early to celebrate 😄

The issue was actually an interesting one: in RLP struct we have two static functions called encode that accept Any and [Any?] as arguments respectively. The issue was that we cast some elements of the [Any?] to [Any] while encoding and calling encode on these elements recursively, and that mistakenly leads to calling func encode(_ element: Any) instead of func encode(_ elements: [Any?]) and it's all just because of the missing ? sign.

To fix that once and for all we now have encode(element: Any) and encode(_ elements: [Any?]). It's impossible to misuse them now.

… requested/decoded - we do not assume that client is using Ethereum Mainnet! According to EIP-681: chain_id is optional and contains the decimal chain ID, such that transactions on various test- and private networks can be requested. If no chain_id is present, the client's current network setting remains effective. In our case if no chainID is present we cannot validate ENS address and will not attempt to guess the network.

JeneaVranceanu · 2023-01-14T23:02:03Z

@yaroslavyaroslav @janndriessen I'm done with this PR. Except the issue mentioned in this comment: #725 (comment)

I'll push the fix for that (if there will be a need for any fixes) in a different PR to make it easier to review later.

About the ENS parsing issue: I've pushed an update to this PR nonetheless and below are my final conclusions.

According to EIP-681: chain_id is optional and contains the decimal chain ID, such that transactions on various test- and private networks can be requested. If no chain_id is present, the client's current network setting remains effective.
In our case, if no chainID is present we cannot validate ENS address and will not attempt to guess the network.

yaroslavyaroslav · 2023-01-15T09:17:23Z

and that mistakenly leads to calling func encode(_ element: Any) instead of func encode(_ elements: [Any?]) and it's all just because of the missing ? sign.

I assume that the encode(_ element: [Any]) is what you've meant here in the first case, correct me if I'm wrong.

To fix that once and for all we now have encode(element: Any) and encode(_ elements: [Any?]). It's impossible to misuse them now.

My personal high priority goal for 4.0 would be to move all type erased bits of code to generic one.

I'd made an issue for that soonish.

JeneaVranceanu · 2023-01-15T10:40:51Z

I assume that the encode(_ element: [Any]) is what you've meant here in the first case, correct me if I'm wrong.

To fix that once and for all we now have encode(element: Any) and encode(_ elements: [Any?]). It's impossible to misuse them now.

My personal high priority goal for 4.0 would be to move all type erased bits of code to generic one.

I'd made an issue for that soonish.

No, no, I meant exactly that. The func encode(_ element: Any) was called by mistake because element: Any fits all of these types Any, Any?, [Any] and [Any?]. While we expected that func encode(_ elements: [Any?]) will be called.

Food for thought about generics.

Generics are good but at the moment I doubt that we will be able to implement them in "the standard way" generics are used. Here is a thought exercise so to speak (and maybe while I'll be writing it down it will all result in me realising I was wrong, who knows):

We have quite a lot of different data types to handle. Let's use BigInt, Data and String. Now the question: how do we declare using Swift language function func encode(_ elements: [Any?]) of struct RLP using generics to account for cases when we have elements array containing 2 or more different data types?

Ha ha, I was wrong. We can use the type constraint feature of generics to account for all supported data types. 🤦‍♂️

Okay, so the solution roughly would be:

internal static func encode<T: Web3DataType>(_ elements: [T?]) -> Data? {
    ...
}

Where Web3DataType is the protocol that has (ATM, no other functions seem to be necessary):

/// Binary representation. Used in the ABI encoding process.
func serialize() -> Data

But we still must account for nested arrays, like in the case of RLP.encode. It can legally be [T, T, [T, [T]]] and we must be able to encode it.

The solution for the nested arrays issue could be:
we recommend applying functions like RLP.encode to nested arrays before these are added to the most outer array: [T, T, RPL.encode([T, RLP.encode([T])])]. Note, the RLP.encode is just an example, it could be any function.

yaroslavyaroslav · 2023-01-15T15:19:34Z

how do we declare using Swift language function func encode(_ elements: [Any?]) of struct RLP using generics to account for cases when we have elements array containing 2 or more different data types?

Yep, this is a tough one, i mean any mixed types arrays are a pain in the ass in swift. But it's solvable in case of encoding if we are known the all possible types that we should handle, e.g. if the API are stable, which is our case here. Decoding case is even more simpler than this, again if we know what type we're expecting at the end.

Nothing to say that in particular both of this cases has been solved in new network implementation which was belongs at [Any] before and belongs at generic for now.

I just have not enough effort back then to refactor contract ABI in the same way, but I'd bet it would fit there as well.

Upd1: also worth to say that swift 5.7 provides a solution for exact that cases (scroll to the "Unlock existentials for all protocols"), but I'd suggest to not belongs on it yet, since it's not having backward capability. But in 5.7 the following code are legal:

let tvShow: [any Equatable] = ["Brooklyn", 99]

Upd2: I've got a post right about that https://yaroslavyaroslav.github.io/posts/generic-protocol-workaround/ based on the solution that I've implemented whithin 3.0.0 release.

Upd3: I've read it just yet once more and there's a lot of grammatical errors, sorry for that 🤷🏻‍♂️.

yaroslavyaroslav · 2023-01-19T20:12:53Z

Ah, and about to generic nested arrays, we're handling them already actually. You can look at /Sources/Web3Core/Utility/Decodable+Extensions.swift, this whole file is about to decode such generic nested array, reasonable if you'll look at Encodable+Extensions.swift in the same path (I guess) you will se how we handle Encoding.

This was one of the first serious challenge of mine within this project.

Also it's funny how even here in this open-source project we're loosing our grip of knowledge of the whole codebase, in favor of focusing on some logical piece of it in. I've previously saw it in some large and well decoupled enterprise codebases, where it's certainly expected though.

JeneaVranceanu · 2023-01-22T15:27:25Z

@yaroslavyaroslav Decodable+Extensions.swift is a good example of decoding a single value, an array or an array of arrays. Probably I phrased the issue not properly. The problem is not in the encoding/decoding but it is in the declaration of a function that can accept an array that can hold at the same time elements of different types including nested arrays.

An example.
In Decodable+Extension.self you have 3 main functions:

public func decodeHex<T: DecodableFromHex>(_ type: T.Type, forKey: KeyedDecodingContainer<K>.Key) throws -> T

public func decodeHex<T: DecodableFromHex>(_ type: [T].Type, forKey: KeyedDecodingContainer<K>.Key) throws -> [T]

public func decodeHex<T: DecodableFromHex>(_ type: [[T]].Type, forKey: KeyedDecodingContainer<K>.Key) throws -> [[T]]

the first accepts type value as type T and returns a single value;
the second accepts type value as an array of type T and returns an array;
the second accepts type value as a 2D array of type T and returns a 2D array.

In the case of RLP, the way we have it implemented expects an array that can hold at the same time single values and arrays. It's not T, [T] or [[T]] it's rather [T, [T], [[T]]] and the order is not guaranteed. (Note: our implementation of RLP doesn't handle [[T]], it's just an example).

The issue is - how would you declare such a function with generics?
Without extra (custom) data structures to handle that issue.

JeneaVranceanu · 2023-01-22T15:28:28Z

@yaroslavyaroslav I'll fix conflicts and this PR should be ready for review.

JeneaVranceanu · 2023-01-22T15:39:49Z

Conflicts fixed.

yaroslavyaroslav · 2023-01-22T15:43:14Z

The issue is - how would you declare such a function with generics?
Without extra (custom) data structures to handle that issue.

For exact this given case I suppose we have to rely on 5.7 lang features only. And there we could make a protocol with the associated value that would be in its turn provides a methods to handle a given type. But that's a tough one, there definitely would be some caveats on this path.

JeneaVranceanu · 2023-01-22T15:47:34Z

The issue is - how would you declare such a function with generics?
Without extra (custom) data structures to handle that issue.

For exact this given case I suppose we have to rely on 5.7 lang features only. And there we could make a protocol with the associated value that would be in its turn provides a methods to handle a given type. But that's a tough one, there definitely would be some caveats on this path.

Definitely worth trying. I'll open an issue for that and link our discussion there with some parts copy-pasted and modified to describe the issue and suggested potential solution.

JeneaVranceanu · 2023-01-22T16:23:41Z

@yaroslavyaroslav @janndriessen PR updated, conflicts fixed. Please, when have time review this PR.

JeneaVranceanu · 2023-02-01T16:31:48Z

@janndriessen Please, approve if you have no more comments.

fix: removed the use of AnyObject in all places possible - replaced w…

d5df8e2

…ith Any

JeneaVranceanu requested review from janndriessen and yaroslavyaroslav January 13, 2023 15:34

JeneaVranceanu self-assigned this Jan 13, 2023

JeneaVranceanu commented Jan 13, 2023

View reviewed changes

yaroslavyaroslav reviewed Jan 13, 2023

View reviewed changes

Sources/Web3Core/Transaction/Envelope/EIP1559Envelope.swift Outdated Show resolved Hide resolved

yaroslavyaroslav reviewed Jan 13, 2023

View reviewed changes

Sources/web3swift/Tokens/ERC20/ERC20BaseProperties.swift Outdated Show resolved Hide resolved

Sources/web3swift/Utils/EIP/EIP712.swift Show resolved Hide resolved

JeneaVranceanu added 5 commits January 14, 2023 00:18

fix: ERC20BaseProperties are back to inherit from AnyObject (restrict…

79886de

…ing protocol to classes)

fix: some spacing and docs fixed

6534980

fix: Networks.fromInt must not return optional

15f7770

fix: EIP681 - when parsing arguments chain ID must be defaulted to Ma…

a49f771

…innet instead of 0

JeneaVranceanu commented Jan 14, 2023

View reviewed changes

JeneaVranceanu added 2 commits January 14, 2023 16:44

chore: removed redundant parameters: [] in read operation

af66469

fix: spacings

922da41

JeneaVranceanu commented Jan 14, 2023

View reviewed changes

JeneaVranceanu added 4 commits January 14, 2023 18:06

chore: removed spacing in " )

03f1395

fix: removed as AnyObject from ENS related code

8e92566

chore: replaced [Any]() with [] for default parameter value

c448970

chore: test refactoring - checking for nullability by using XCTAssert…

2bd3303

…NotNil

fix: RLP func encode for single value has named parameter

ed43d2d

It fixes the issue when trying to call func element() with [Any].

chore: merge with develop

f6c1805

chore: EIP681 docs + more descriptive log message

be6c585

JeneaVranceanu requested a review from yaroslavyaroslav January 22, 2023 16:22

JeneaVranceanu mentioned this pull request Jan 22, 2023

Implementing proper generics for ABI encoding/decoding #738

Open

yaroslavyaroslav approved these changes Jan 31, 2023

View reviewed changes

JeneaVranceanu mentioned this pull request Feb 1, 2023

feat: convertToData handles Bool #750

Merged

janndriessen approved these changes Feb 2, 2023

View reviewed changes

JeneaVranceanu merged commit 638fa77 into web3swift-team:develop Feb 2, 2023

JeneaVranceanu deleted the fix/anyobject-usage-removed branch February 2, 2023 15:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: replaced AnyObject with Any #725

fix: replaced AnyObject with Any #725

JeneaVranceanu commented Jan 13, 2023 •

edited

JeneaVranceanu commented Jan 13, 2023

JeneaVranceanu Jan 13, 2023

yaroslavyaroslav Jan 13, 2023

JeneaVranceanu Jan 13, 2023

JeneaVranceanu Jan 13, 2023

JeneaVranceanu Jan 13, 2023

yaroslavyaroslav Jan 31, 2023

JeneaVranceanu Jan 14, 2023

JeneaVranceanu Jan 14, 2023

JeneaVranceanu Jan 14, 2023

JeneaVranceanu Jan 14, 2023

JeneaVranceanu commented Jan 14, 2023 •

edited

yaroslavyaroslav commented Jan 14, 2023 •

edited

JeneaVranceanu commented Jan 14, 2023

JeneaVranceanu commented Jan 14, 2023

yaroslavyaroslav commented Jan 15, 2023 •

edited

JeneaVranceanu commented Jan 15, 2023

yaroslavyaroslav commented Jan 15, 2023 •

edited

yaroslavyaroslav commented Jan 19, 2023 •

edited

JeneaVranceanu commented Jan 22, 2023

JeneaVranceanu commented Jan 22, 2023

JeneaVranceanu commented Jan 22, 2023

yaroslavyaroslav commented Jan 22, 2023

JeneaVranceanu commented Jan 22, 2023

JeneaVranceanu commented Jan 22, 2023

JeneaVranceanu commented Feb 1, 2023

fix: replaced AnyObject with Any #725

fix: replaced AnyObject with Any #725

Conversation

JeneaVranceanu commented Jan 13, 2023 • edited

Summary of Changes

Test Data or Screenshots

By submitting this pull request, you are confirming the following:

JeneaVranceanu commented Jan 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JeneaVranceanu commented Jan 14, 2023 • edited

yaroslavyaroslav commented Jan 14, 2023 • edited

JeneaVranceanu commented Jan 14, 2023

JeneaVranceanu commented Jan 14, 2023

yaroslavyaroslav commented Jan 15, 2023 • edited

JeneaVranceanu commented Jan 15, 2023

Food for thought about generics.

yaroslavyaroslav commented Jan 15, 2023 • edited

yaroslavyaroslav commented Jan 19, 2023 • edited

JeneaVranceanu commented Jan 22, 2023

JeneaVranceanu commented Jan 22, 2023

JeneaVranceanu commented Jan 22, 2023

yaroslavyaroslav commented Jan 22, 2023

JeneaVranceanu commented Jan 22, 2023

JeneaVranceanu commented Jan 22, 2023

JeneaVranceanu commented Feb 1, 2023

JeneaVranceanu commented Jan 13, 2023 •

edited

JeneaVranceanu commented Jan 14, 2023 •

edited

yaroslavyaroslav commented Jan 14, 2023 •

edited

yaroslavyaroslav commented Jan 15, 2023 •

edited

yaroslavyaroslav commented Jan 15, 2023 •

edited

yaroslavyaroslav commented Jan 19, 2023 •

edited