Web3.swift icon indicating copy to clipboard operation
Web3.swift copied to clipboard

Thoughts on web3 PromiEvent

Open pixelmatrix opened this issue 6 years ago • 3 comments

This is less of an issue, but more of a question. I'm wondering how you're thinking about handling the equivalent of web3.js' PromiEvent (promise + event emitter). More specifically, I'm curious how this Web3 library will handle callbacks related to transaction events. In Javascript, you can get called when your transaction receipt is ready, when it's confirmed, and if it fails, which is essential for the dapp experience.

Personally, I think the PromiEvent thing they're doing is a bit of a strange pattern, and there are cleaner patterns in Foundation that we can borrow from. Two options that come to mind are the delegate pattern and NotificationCenter. Have you put any thought into this area yet?

In my own prototype I've built a small TransactionWatcher class that takes a transaction hash, polls the network and fires notifications based on the status of the transaction. I'd be happy to share that code when it's ready, or adopt a different way if you already have plans around this.

pixelmatrix avatar May 15 '18 06:05 pixelmatrix

I honestly didn't even know this PromiEvent existed but there is definitely a use case for it. It would actually simplify some stuff I'm doing right now in some of my own projects.

Your idea sounds promising but please don't use the NotificationCenter if you can avoid it as I don't really like this pattern.
I have some additional thoughts for you:

  • Do you know how web3.js polls changes to the transaction? In our library we have the option to communicate to the Ethereum node in any way you want (IPC, HTTP, ?, ...) but most of the time it will be over HTTP. We have to minimize the number of requests as much as possible and still need to detect the changes as fast as possible.
  • At what time do we stop making requests if something goes wrong and we don't ever reach the 'receipt' or 'confirmation' state? We need to limit the total time to wait or the total number of requests.
  • Could we somehow still integrate it with PromiseKit such that we can chain all transaction based stuff something like in the following example?
firstly {
    web3.eth.sendRawTransaction(transaction: tx)
}.then { hash in
    TransactionWatcher(hash: hash).promise
}.done { watcher in
    watcher.receipt { receipt in
        print(receipt)
    }
    watcher.confirmation { confirmation in
        print(confirmation)
    }
}.catch { error in
    print(error)
}

Or something similar...

What do you think?

koraykoska avatar May 15 '18 16:05 koraykoska

The way web3.js does this is unfortunately not great for regular HTTPProvider, but it gets the job done. The code is here: https://github.com/ethereum/web3.js/blob/1.0/packages/web3-core-method/src/index.js#L244

The first step for all provider types is that they poll for the transactionReceipt. Then, from what I can tell, if you're on a socket connection, they subscribe to new blocks, then check the receipt again when one comes back, looking for status and gasUsed. On HTTP connections, they simply check the receipt every 1 second. They call this "confirmations" but they're actually not checking the block number on http, so it's not a true confirmation.

In my own watcher class (we use HTTP), I'm calling getTransactionReceipt() on a loop until it's received, then calling blockNumber() on a loop. I'm currently polling every 2 seconds. When the block number is incremented, I then request the transaction receipt again and check for errors. After a defined amount of blocks with no failure, I then mark the transaction as successful and stop polling. I've heard 6 confirmations is generally good enough for most transactions. This is exposed through a status enum that gets moved from pending -> confirmed(times: Int) -> successful (or failed), and broadcast to the app via a Notification.

In terms of failure, I'm not sure where we should stop. If the receipt is ever marked as failed, it doesn't make sense to keep checking, but if there's no receipt that's harder to reason about because it can take a long time for a transaction to be picked up. It probably also makes sense to account for network failures as well given that it's a mobile sdk. Perhaps there should be a timeout or at least a back-off if the receipt is not received after a certain amount of time. It may be something that individual dapps want to configure on a per transaction basis as well.

Regarding NotificationCenter, I also don't really like the API, but one benefit of it is that it's a to-many event system similar to the events in web3.js. For most of my use cases a delegate pattern would make a lot of sense, but that precludes you from watching for confirmations somewhere else. I don't feel too strongly either way, but wanted to give you that context.

As far as PromiseKit, I did a little research to see if there's any precedent for this. It sounds like they want to avoid this pattern, at least for the confirmations: https://github.com/mxcl/PromiseKit/blob/master/Documentation/FAQ.md#i-need-my-then-to-fire-multiple-times

That being said, I like how clean your example reads. I think getting the initial receipt could easily be a promise, but the confirmations thing is a bit weird. Perhaps it could be another promise with NSProgress? Lots of possibilities here.

pixelmatrix avatar May 15 '18 18:05 pixelmatrix

Your implementation sounds quite reasonable and I think it should also work with socket connections for now.

I get now what you mean with confirmation. Actual block confirmations. I think 6 is more than enough but this should be definitely configurable.

To avoid the NotificationCenter we could still add the possibility to register multiple callbacks in our class. I know that's not the same but like I said I want to avoid it as much as possible. If it still seems as the most viable solution we could give it a try though.

I am not yet sure how to integrate PromiseKit in this specific case but if you have any ideas just let me know. Maybe implementing it without Promises for now would be the best. We can worry later about possible PromiseKit extensions.

koraykoska avatar May 15 '18 22:05 koraykoska