Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to decode Dynamic array. Potential Int overflow in the decoder for dynamic array. #150

Open
metallicalfa2 opened this issue Jul 28, 2021 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@metallicalfa2
Copy link

metallicalfa2 commented Jul 28, 2021

This is my struct.

struct TupleWithArray: ABITuple {
    static var types: [ABIType.Type] { [ABIArray<EthereumAddress>.self] }
    
    var owners: [EthereumAddress]
    
    init(owners: [EthereumAddress]) {
        self.owners = owners
    }
    
    init?(values: [ABIDecoder.DecodedValue]) throws {
        self.owners = try values[0].decodedArray()
    }
    
    func encode(to encoder: ABIFunctionEncoder) throws {
        try encoder.encode(owners)
    }
    
    var encodableValues: [ABIType] { [ABIArray(values: owners)] }
}

Here's the test

 do {
            let tuple = TupleWithArray(owners: [EthereumAddress("0xdF136715f7bafD40881cFb16eAa5595C2562972b"), EthereumAddress("0xdF136715f7bafD40881cFb16eAa5595C2562972b")])
            
            let encodedHexString = try encoder.encoded().web3.hexString
            
            // Fails
            let value = try ABIDecoder.decodeData(encodedHexString, types: [TupleWithArray.self])
            
        } catch {
            XCTFail()
        }

The code fails in ABIDecoder.swift during Int(hex: string) due to overflow.

guard var size = Int(hex: sizeHex) else {
                throw ABIError.invalidValue
            }

// The Following is nil

    let a = Int.init("000000000000000000000000e000000000000000000000000000000000000000", radix: 16)
@metallicalfa2
Copy link
Author

@DarthMike

@DarthMike
Copy link
Member

DarthMike commented Jul 30, 2021

Hello @rathishubham7, I think you need to call decodedTupleArray() instead of decodedArray() (unfortunately it's necessary to allow for static types in the struct when decoding).

We have unit tests for these cases, here.

@DarthMike
Copy link
Member

Just checked again, I think I didn't understand the example, ignore comment above, checking it in more detail.

@DarthMike
Copy link
Member

@rathishubham7 it seems there's indeed a bug with tuples containing only arrays; test above is not 100% correct though (need to remove the encoded function signature)

func test_GivenTupleWithDynamicArray_EncodesCorrectly() {

        let tuple = TupleWithArray(owners: [EthereumAddress("0xdF136715f7bafD40881cFb16eAa5595C2562972b"), EthereumAddress("0xdF136715f7bafD40921cFb16eAa5595C2562972b")])

        try? encoder.encode(tuple)

        let encoded = try? encoder.encoded().web3.hexString

        XCTAssertEqual(encoded, "0xdf7116a2000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000002000000000000000000000000df136715f7bafd40881cfb16eaa5595c2562972b000000000000000000000000df136715f7bafd40921cfb16eaa5595c2562972b")

        let value = try? ABIDecoder.decodeData(encoded?.replacingOccurrences(of: "df7116a2", with: "") ?? "", types: [TupleWithArray.self])

        let decodedTuple: TupleWithArray? = try? value?[0].decoded()

        XCTAssertEqual(decodedTuple?.owners.first, EthereumAddress("0xdF136715f7bafD40881cFb16eAa5595C2562972b"))
        XCTAssertEqual(decodedTuple?.owners.last, EthereumAddress("0xdF136715f7bafD40921cFb16eAa5595C2562972b"))

    }

Can workaround this issue by changing the decoder intializer.

init?(values: [ABIDecoder.DecodedValue]) throws {
        self.owners = try values.map { try $0.decoded() }
    }

@DarthMike
Copy link
Member

We'll fix the bug, thanks for the report 🙌

@metallicalfa2
Copy link
Author

Thanks @DarthMike. I'll take a look at the workaround as well.

@metallicalfa2
Copy link
Author

Hi @DarthMike. Any updates ?

The workaround will not work in the following case, right ?

struct TupleWithArray: ABITuple {
    static var types: [ABIType.Type] { [ABIArray<EthereumAddress>.self, BigUInt.self] }
}```

@DarthMike
Copy link
Member

Hello, we're very busy with other work, so we're not prioritizing fixing this soon, will try to look at it this week and check if it's a simple fix.

You can still workaround for that case, I've written a test to demonstrate it:

func test_GivenTupleWithArrayAndNumber_EncodesCorrectly() {

            let tuple = TupleWithArrayAndNumber(owners: [EthereumAddress("0xdF136715f7bafD40881cFb16eAa5595C2562972b"), EthereumAddress("0xdF136715f7bafD40921cFb16eAa5595C2562972b")],
                                                value: 98
                                       )

            try? encoder.encode(tuple)

            let encoded = try? encoder.encoded().web3.hexString

            XCTAssertEqual(encoded, "0x7a90ff650000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000620000000000000000000000000000000000000000000000000000000000000002000000000000000000000000df136715f7bafd40881cfb16eaa5595c2562972b000000000000000000000000df136715f7bafd40921cfb16eaa5595c2562972b")

            let value = try? ABIDecoder.decodeData(encoded?.replacingOccurrences(of: "7a90ff65", with: "") ?? "", types: [TupleWithArrayAndNumber.self])

            let decodedTuple: TupleWithArrayAndNumber? = try? value?[0].decoded()

            XCTAssertEqual(decodedTuple?.owners.first, EthereumAddress("0xdF136715f7bafD40881cFb16eAa5595C2562972b"))
            XCTAssertEqual(decodedTuple?.owners.last, EthereumAddress("0xdF136715f7bafD40921cFb16eAa5595C2562972b"))
            XCTAssertEqual(decodedTuple?.value, 98)

        }

With the tuple written as:

struct TupleWithArrayAndNumber: ABITuple, Equatable {
    static var types: [ABIType.Type] { [ABIArray<EthereumAddress>.self, BigUInt.self] }

    var owners: [EthereumAddress]
    var value: BigUInt

    init(owners: [EthereumAddress],
         value: BigUInt) {
        self.owners = owners
        self.value = value
    }

    init?(values: [ABIDecoder.DecodedValue]) throws {
        self.owners = try values.dropLast().map { try $0.decoded() }
        self.value = try values[values.count - 1].decoded()
    }

    func encode(to encoder: ABIFunctionEncoder) throws {
        try encoder.encode(owners)
        try encoder.encode(value)
    }

    var encodableValues: [ABIType] { [ABIArray(values: owners), value] }
}

The important workaround part is:

init?(values: [ABIDecoder.DecodedValue]) throws {
        self.owners = try values.dropLast().map { try $0.decoded() }
        self.value = try values[values.count - 1].decoded()
    }

@DarthMike DarthMike added the bug Something isn't working label Dec 23, 2021
@DarthMike DarthMike self-assigned this Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants