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

topUpBatch return Promise<void> while it should be Promise<BatchID> #975

Open
mahiarirani opened this issue Jan 23, 2025 · 1 comment · May be fixed by #977
Open

topUpBatch return Promise<void> while it should be Promise<BatchID> #975

mahiarirani opened this issue Jan 23, 2025 · 1 comment · May be fixed by #977

Comments

@mahiarirani
Copy link

Looking at the topUp functions, it’s anticipated to return void while invoking the stamps.topUp function, which is expected to return a Batch ID.

  • Could you please explain the rationale behind having it return void?
  • Additionally, how can I determine the success of my topUp operation?
  • Is it possible to modify the return value to match the API result or come close to it?
@Cafe137
Copy link
Collaborator

Cafe137 commented Feb 7, 2025

Hello @mahiarirani,

Thank you for your interest in the project.

Could you please explain the rationale behind having it return void?

It is certainly an oversight. There is no reason not to return BatchId in the response. However, do note that the BatchId does not change after the topup or dilute operations. It should be the same BatchId that you pass to these functions.

Additionally, how can I determine the success of my topUp operation?

You can monitor GET /transactions via the getAllPendingTransactions method to wait for all transactions to finish. After that, your Bee node should pick up the new batch values for your postage batch. You will not see the new amount on your batch right after the topup operation finishes, but it should update to the new values within a minute or two.

Is it possible to modify the return value to match the API result or come close to it?

Yes, I am linking a PR soon that will fix it.

Cafe137 added a commit that referenced this issue Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@mahiarirani @Cafe137 and others