decentraland / decentraland-eth

DEPRECATED - Ethereum common helpers for Decentraland
https://decentraland.github.io/decentraland-eth/
Apache License 2.0
15 stars 9 forks source link

feat: dont fail in connect using only a providerUrl #13

Closed menduz closed 6 years ago

menduz commented 6 years ago

The client is using this inside a WebWorker, that means we have no instance of web3/metamask there, and we only rely on an URL provider

menduz commented 6 years ago

Do we really want to separate two similar classes one for accounts and another one for read-only access? I like the idea of removing eth.ts tho.

I think it would be comfy to have only one class with a method called hasAddress(): boolean to verify if we have signing power instead of the hard throw of getAccount()

eordano commented 6 years ago

It's a very common use case to have a provider for read-only data and another provider for writing/signing.

Specially, because the ledger/trezor/metamask do not store the blockchain (they redirect to a remote node for queries).

If we have a faster node it makes sense to not hit the signing provider. I think a better web3 architecture would make this more evident, it looks like the current multiple-provider approach is a leaky abstraction.

menduz commented 6 years ago

What about getting rid of the connect function and only instantiate contracts with providers?

const mana = new MANAToken(manaAddress)

const provider = new ProviderEngine()
provider.addProvider(new LedgerHwProvider())
provider.addProvider(new WebSocketProvider(url))

mana.setProvider(provider)

@eordano @NicoSantangelo

nicosantangelo commented 6 years ago

That makes sense, but we still need to be able to access web3 for things like getTransaction.

(I'll merge this PR to unblock development, we can discuss it further here or elsewhere)