Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

New WebsocketProvider and Subscriptions implementation#446

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed

Conversation

@odanylovych
Copy link

Hi.

First of all, thanks for the library!

We are working on the Avalanche library and we want to use this library as a submodule for Avalanche Web3 implementation, but right now we can't integrate it properly.

To fix this we plan to do 2 PRs to your library. They will decouple networking and subscription parts, so they can be replaced with our implementations. This is the first PR related to networking.

In this PR:

  • We moved subscription logic to the new protocol and implemented it in the WebSocket provider.
  • Infura related logic moved from WebsocketProvider to InfuraWebsocketProvider.
  • WebSocket provider now supports sendAsync and subscribe methods at the same time so can be used as a JSON RPC call provider too.
  • Added filter and subscription methods to the Eth object. Subscription methods will return an error if a provider isn't supporting subscriptions.

If you need more info, please, tell us. Will be glad to provide it.

Thanks.

JeneaVranceanu reacted with thumbs up emoji
case.newBlockFilter:
return0
case.getFilterLogs:
returnnil
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

What is the difference betweennil and0?
Looks like the meaning is the same but nowrequiredNumOfParameters has to be unwrapped.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

nil means that there are variable number of parameters

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Could you please add a documentation comment torequiredNumOfParameters mentioning that explanation?

Copy link
Author

@odanylovychodanylovychMay 19, 2022•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Added comment torequiredNumOfParameters in54cf7e9

return valuesas?T
}
guardlet value=self.valueas?Telse{returnnil}
return value
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Minor improvement:return self.value as? T without guard statement.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

done

JeneaVranceanu reacted with thumbs up emoji
@JeneaVranceanu
Copy link
Collaborator

@odanylovych Great job! Will fork it and try it out. Could you please update thedocumentation part related to the websockets?

publiclettransactionsRoot:String
}

publicstructLogItem:Decodable{
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This struct duplicatesEventLog atthe line 331.

I've tested this branch and was able to get the same result usingEventLog instead ofLogItem.

For the sake of interoperability I think it will be better to useEventLog forweb3.web3contract.getIndexedEvents,web3.web3contract.getIndexedEventsPromise and for the new subscription API. Especially considering that these structs do not havepublic init (e.g. t won't be convenient to createEventLog out ofLogItem) to avoid duplicates on the side of developers using this library it will make more sense to useEventLog as a single model for logs whether it's from web socket or not.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Example input (web socket message received) and output (decoded):

received text: {   "jsonrpc":"2.0",   "method":"eth_subscription",   "params":{      "result":{         "address":"0x6eb84ef768ffe65bca008af0dea3175eaf4df2ed",         "blockHash":"0x6474699c14877b8b614c9ed417295913b283158a29964943f90c1802e6d1320a",         "blockNumber":"0xbcf944",         "data":"0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000014414a6e2930000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000015ef83ad9559033e6e941db7d7c495acdce616347d28e90c7ce47cbfcfcad3bc50000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000596f357c6a7b0ce6b9327b83166db09820ec6703e1a1ff0c769747199f59130abc9c314ad5697066733a2f2f516d51474369465539657258756175396752537234695a48766b6a6e57636576625968784d324462664e47486f550000000000000000000000000000000000000000000000000000000000000000000000",         "logIndex":"0x1",         "removed":false,         "topics":[            "0x2e733a17851169f232b3859260eb3ad2a086afd54e999eb4ea9afb7791702e41",            "0x0000000000000000000000000000000000000000000000000000000000000000"         ],         "transactionHash":"0x12b9f9312bfac44b95c82cf139a4a102efbe784a38e4eb973d1397a8b0f34a0a",         "transactionIndex":"0x0",         "transactionLogIndex":"0x1",         "type":"mined"      },      "subscription":"0x451a9281a0d8a6a4"   }}
EventLog(address: web3swift.EthereumAddress(_address: "0x6eb84ef768ffe65bca008af0dea3175eaf4df2ed", type: web3swift.EthereumAddress.AddressType.normal), blockHash: 32 bytes, blockNumber: 12384580, data: 416 bytes, logIndex: 1, removed: false, topics: [32 bytes, 32 bytes], transactionHash: 32 bytes, transactionIndex: 0)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Resolved in2d90894
Thanks@odanylovych !

publicletstateRoot:String
publiclettimestamp:String
publiclettransactionsRoot:String
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This model should be updated.
These attributes should be optional:

    public let logsBloom: String    public let nonce: String    public let number: String

Andpublic let hash: String? should be added.

Source fromweb3js library.
Also inEthereum-Go repo I can see thatnonce is also optional but other fields are required which is confusing...

Copy link
Collaborator

@JeneaVranceanuJeneaVranceanuFeb 1, 2022•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Here is an example of receiving new heads.

{   "jsonrpc":"2.0",   "method":"eth_subscription",   "params":{      "result":{         "author":"0xcc271fdb440c0a37c51a770f415521d0e95a5518",         "difficulty":"0xfffffffffffffffffffffffffffffffd",         "extraData":"0xde830207028f5061726974792d457468657265756d86312e34312e30826c69",         "gasLimit":"0x2faf080",         "gasUsed":"0x0",         "hash":"0x53ce7a56293bc8e93d85abeb1c342bdf7ae1cc191aa0237d853f73ed1899a8e2",         "logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000",         "miner":"0xcc271fdb440c0a37c51a770f415521d0e95a5518",         "number":"0xbd1c06",         "parentHash":"0xa30928f385aa798a74fef9079d9579a3400c50ff2e6cfbd80bcd0302c556f75b",         "receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",         "sealFields":[            "0x139840c4",            "0xf232e5ae28957588aac4f5556785ffeb7f9246c7277bc06c5e5aea6c933f5e52260faa2d9c016844c38dd3b3f8190ba1d08d23fe87c576b7d1b85a99e11a07f101"         ],         "sha3Uncles":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",         "signature":"f232e5ae28957588aac4f5556785ffeb7f9246c7277bc06c5e5aea6c933f5e52260faa2d9c016844c38dd3b3f8190ba1d08d23fe87c576b7d1b85a99e11a07f101",         "size":"0x248",         "stateRoot":"0xadf409adae4774a2171a702f4441c5669b798eb0113fc8765c891ebf3c6c7bb1",         "step":"328745156",         "timestamp":"0x61f943d4",         "transactionsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421"      },      "subscription":"0x1364cf3b35e2605f"   }}

Copy link
Collaborator

@JeneaVranceanuJeneaVranceanuFeb 1, 2022•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

As an alternative, I suggest modifying this function to accept a generic type:https://github.com/skywinder/web3swift/pull/446/files#diff-9743ac53f1b42c7462dbf4d5dde6f2f434845bf1bdc66c25858bf6b4cdacf0f9R27

Depending on the network used model can be specified and it will be up to the user of this library.

Copy link
Collaborator

@yaroslavyaroslavyaroslavyaroslavMar 16, 2022•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I'm also researching this part of the lib right now and i've confused either.

In web3js regarding to the docs these 4 fields are optional, in the ether.js all fields are non optional.

More than that in Ethereum github JSON-RPC schema all of the fields are marked as required so there's should not be optional at all.

Speaking about go implementation, as i saw their code it seems that explicitly optional checking for external API struct fields are best practices for that language, which is not in swift.

@yaroslavyaroslav
Copy link
Collaborator

@odanylovych hi, due to last merged PR in development branch your one have some conflicts, could you please resolve it.

Also it seems that ci/cd pipeline fails on carthage building, fix it please.

JeneaVranceanu reacted with thumbs up emojiJeneaVranceanu reacted with eyes emoji

@JeneaVranceanu
Copy link
Collaborator

JeneaVranceanu commentedMar 17, 2022•
edited
Loading

@odanylovych hi, due to last merged PR in development branch your one have some conflicts, could you please resolve it.

Also it seems that ci/cd pipeline fails on carthage building, fix it please.

Also looking forward for it to be reviewed and if accepted - merged. It's a good update to the library.

@yaroslavyaroslav
Copy link
Collaborator

Just saying: You're able to run CI/CD pipeline within your fork following by thishint.

Also please take a look at PR rules, to make sure that you're passing all the points.

@odanylovych
Copy link
Author

@odanylovych Great job! Will fork it and try it out. Could you please update thedocumentation part related to the websockets?

Updatedthe documentation

@yaroslavyaroslav
Copy link
Collaborator

yaroslavyaroslav commentedMay 25, 2022•
edited
Loading

@odanylovych sorry for it taking so long to review your pr.

Just saying, that currently i'm focused on 3.0.0-RC02 release which will include almost totally rewritten http network layer.

I'll be able to review your pr afterwards, but please be noticed, that kindly unlikely that there'll be any 2.7.0 release to where could be included some massive change, rather then 3.0.0 because of kindly likely huge problems with merging that updates into 3.0.0 branch, but that's not for sure, because 3.0.0 haven't touches any Websocket code parts yet.

@odanylovych
Copy link
Author

@odanylovych sorry for it taking so long to review your pr.

Just saying, that currently i'm focused on 3.0.0-RC02 release which will include almost totally rewritten http network layer.

I'll be able to review your pr afterwards, but please be noticed, that kindly unlikely that there'll be any 2.7.0 release to where could be included some massive change, rather then 3.0.0 because of kindly likely huge problems with merging that updates into 3.0.0 branch, but that's not for sure, because 3.0.0 haven't touches any Websocket code parts yet.

No problem 👍

Comment on lines -10 to -28
publicenumBlockNumber{
case pending
case latest
case earliest
case exact(BigUInt)

publicvarstringValue:String{
switchself{
case.pending:
return"pending"
case.latest:
return"latest"
case.earliest:
return"earliest"
case.exact(let number):
returnString(number, radix:16).addHexPrefix()
}
}
}
Copy link
Collaborator

@yaroslavyaroslavyaroslavyaroslavJun 6, 2022•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I've just started to reviewing this PR, thinking it could take a while (like up to the end of this week).

But just yet i have question like what's the purpose of this droppingBlockNumber enum?

I see that you've making an abstraction over the API spec, which is more close to given enum that to any literal type itself.

@ypopovych
Copy link

@yaroslavyaroslav do you have any news about the 3.0 version? We will update our PRs

@yaroslavyaroslav
Copy link
Collaborator

yaroslavyaroslav commentedJul 6, 2022•
edited
Loading

@ypopovych thank you for asking. We're done with implementing new network layer for http part. Please take a look atunstable branch and especially to3.0.0-RC2 release.

@yaroslavyaroslav
Copy link
Collaborator

Sorry to we failed to pass this PR through our pipeline for so long. Really bad time it was for that. So I'm closing it for now, it you'll get some effort to start again I'll be happy to help, I mean really help, since there wouldn't be such dramatic changes in lib in foreseeable feature.

@JeneaVranceanu
Copy link
Collaborator

JeneaVranceanu commentedDec 4, 2022•
edited
Loading

@yaroslavyaroslav@ypopovych@odanylovych
We recently dropped support of WebSockets as a part of major refactoring.
For the last 4-5 months I've been using this branch in my fork of web3swift and it did work well. But I'd like us to move away from Starscream and start using API provided from Apple -https://developer.apple.com/documentation/foundation/urlsessionwebsockettask.

Probably, as the first version of this feature, it will be implemented in such a way that only Apple's WebSocket API will be available but later we could provide an abstraction where it will be the choice between the default implementation that we provide and the implementation provided by the user of the library.

I'll use this branch and will build new add the required modifications.
The plan is to complete this by the end of next week December 11th 2022.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@yaroslavyaroslavyaroslavyaroslavyaroslavyaroslav left review comments

@JeneaVranceanuJeneaVranceanuJeneaVranceanu left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@odanylovych@JeneaVranceanu@yaroslavyaroslav@ypopovych
[8]ページ先頭
©2009-2025 Movatter.jp