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

Consider removal of batch.js #3603

Closed
miohtama opened this issue Jun 28, 2020 · 6 comments
Closed

Consider removal of batch.js #3603

miohtama opened this issue Jun 28, 2020 · 6 comments
Labels
Enhancement Includes improvements or optimizations

Comments

@miohtama
Copy link
Contributor

Expected behavior

Some old code was pointed out by a contributed on Gitter channel:

https://github.com/ethereum/web3.js/blob/a5e832fc20088cf4c4e916bc0ec3107ceb4428d0/packages/web3-core-requestmanager/src/batch.js

This is for doing multiple API calls parallel from 2015.

Actual behavior

Modern JavaScript has similar functionality with Promise.all and some other Promise resolvers. Just make all your API calls as promises and then wait for all of these Promises to resolve once.

I would suggest deprecating this code and then removing it in the next major release.

Here is also an alternative if you want to have the responses, e.g. the call return values, recorded on an object:

https://stackoverflow.com/questions/29292921/how-to-use-promise-all-with-an-object-as-input/51722274#51722274

@DeepalUsyD
Copy link

DeepalUsyD commented Jun 29, 2020

just figured that batch.add has a callback. So I believe this shouldn't be an issue. Besides, batch.add and batch.execute can be used to send a batch of transactions to a blockchain, so I think it's important that it remains in subsequent releases.

@GregTheGreek
Copy link
Contributor

GregTheGreek commented Jun 29, 2020

Do you mind linking the gitter conversation if possible? Found it.

Hmm, yes I think we could probably remove this all together.

@GregTheGreek GregTheGreek added the Enhancement Includes improvements or optimizations label Jul 1, 2020
@DeepalUsyD
Copy link

Do you mind linking the gitter conversation if possible? Found it.

Hmm, yes I think we could probably remove this all together.

Hi,
I am the person who raised this on Gitter. I am wondering, could you please consider not removing this? batch.add and batch.execute has been useful so far for me.

@GregTheGreek
Copy link
Contributor

@DeepalUsyD Would require some further discussions. But looking at it, I can't imagine why you would need this over Promise.all or similar?

@DeepalUsyD
Copy link

DeepalUsyD commented Jul 13, 2020

@GregTheGreek
I am not sure if I am correct. But I see a difference in batch requests and Promise.all
e.g.

let promises =  [];
  for (let i = 0; i < 9000; i++) {
    promises.push(web3.eth.sendTransaction({
      from: "0x8fd0c5f80785d7d13c49d0ca8c88d2fb0803690b",
      to: toAccount,
      value: weiVal
    }));
  }

  return Promise.all(promises);
}

Returns promises for all the pushed transactions but the transactions are pushed in a loop, one after the other. Not as a batch in one go.
In contrast batch.add creates a batch first and executes the created batch. Doesn't push tx one after the other like promise.push:

for (i = 0; i < 3000; i++) {
        batch.add(web3.eth.sendTransaction({
            from: account-address1,
            to: account-address2,
            value: i
        }))
    }
    batch.execute();

I think creating a batch first and executing it later may come in handy. Anyway, I do believe, batch.js does require more explanation in the documentation.

Edit by @GregTheGreek: I formatted the code blocks for readability.

@GregTheGreek
Copy link
Contributor

Ah you know what, you are right. I completely forgot that json-rpc supports a batch method.

I think we can close this, batch.js is here to stay. that being said, adding documentation around this is needed. Will be opening a relevant issue :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Includes improvements or optimizations
Projects
None yet
Development

No branches or pull requests

3 participants