Don't use Parity-specific JSONRPCs #56
Conversation
| @@ -15,213 +15,296 @@ | |||
| // along with Parity. If not, see <http://www.gnu.org/licenses/>. | |||
| const PubsubBase = require('../pubsubBase'); | |||
|
|
|||
| const { inAddress, inBlockNumber, inHex, inNumber16, inOptions, inFilter } = require('../../format/input'); | |||
| const { outAddress, outBlock, outNumber, outTransaction, outSyncing, outReceipt, outLog } = require('../../format/output'); | |||
| const { | |||
There was a problem hiding this comment.
file diff mostly due to prettier :(
| 'eth_getBlockByNumber', | ||
| options | ||
| ).pipe( | ||
| withLatestFrom(onSyncingChanged$(options).pipe(startWith(false))), |
There was a problem hiding this comment.
I might be wrong but wouldn't this make onEveryBlockWithApi$ emit on initialization, even though the user is not synced? e.g.:
- onEveryBlockWithApi$ is called
- blockNumber RPC answers
- onEveryBlockWithApi$ emits wit the blockNumber
- isSyncing RPC answers
true(which was always the case) - onEveryBlockWithApi$ doesn't emit until isSyncing RPC answers
false
There was a problem hiding this comment.
Hmm, I think you're right. The problem is: when you connect to Metamask or infura, eth_subscribe('syncing') doesn't return anything, I assume they're always considered sync. So onSyncingChanged$ actually never fires.
Between the 2, i still prefer to fire once at the beginning.
However I'm open to other ideas
There was a problem hiding this comment.
I imagine a fix could be to make onSyncingChanged$ fire eth_syncing (non-pubsub rpc call) first to get an initial value, and then switch to the pubsub. non-pubsub "eth_syncing" does get an answer.
obviously not very clean though
There was a problem hiding this comment.
I actually think it's a good idea, I thought about it too.
Another reason why it's good: before, parity_subscribe('eth_getBlock') immediately gives you an answer, and then gives you subsequent pubsub answers on each pub. eth_subscribe('newHeads') however doesn't give you the initial one, so there might be a (up to 15s) delay between subscribe and first event fired.
| const [fallbackNamespace, fallbackMethod] = fallback.split('_'); | ||
|
|
||
| return timer(0, pollInterval).pipe( | ||
| switchMap(() => api[fallbackNamespace][fallbackMethod]()) |
There was a problem hiding this comment.
might not be a problem but for RPC calls taking >1 second to answer, the observable would never fire
There was a problem hiding this comment.
Ah yeah, it's a problem. I'll change it to mergeMap.
Is there something between mergeMap and switchMap? I.e. to make this work: https://codesandbox.io/s/9153w6n7r. but not in the mergeMap way (hope it's clear...)
There was a problem hiding this comment.
I think exhaustMap should do the trick https://repl.it/repls/BewitchedSubtleCookies
We have a timer (source observable) ; we make the RPC query each time the timer fires, unless the previous RPC hasn't resolved yet
Use
eth_subscribe('syncing')eth_subscribe('newHeads')instead of
parity_subscribe('eth_syncing')parity_subscribe('eth_blockNumber)In any case, if pubsub doesn't work, default to polling.
Notes: on parity-ethereum nodes,
eth_subscribe('syncing')is not implemented, so it will poll.Fixes #51, fixes #3