Skip to content

Conversation

@PatStiles
Copy link
Contributor

SDK Changes

Description

Makes some changes to sdk:

  • Derives batcher_url from Network paramater.
  • Adds --private_key flag to DepositToBatcher
  • Adds msg to batcher errors telling a user there funds have not been spent.

Type of change

Please delete options that are not relevant.

  • New feature
  • Bug fix
  • Optimization
  • Refactor

Checklist

  • “Hotfix” to testnet, everything else to staging
  • Linked to Github Issue
  • This change depends on code or research by an external entity
    • Acknowledgements were updated to give credit
  • Unit tests added
  • This change requires new documentation.
    • Documentation has been added/updated.
  • This change is an Optimization
    • Benchmarks added/run
  • Has a known issue
  • If your PR changes the Operator compatibility (Ex: Upgrade prover versions)
    • This PR adds compatibility for operator for both versions and do not change batcher/docs/examples
    • This PR updates batcher and docs/examples to the newer version. This requires the operator are already updated to be compatible

Copy link
Contributor

@uri-99 uri-99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

left some comments

@MarcosNicolau
Copy link
Member

@uri-99, your comments have been addressed at: 2e8465c.

@uri-99 uri-99 changed the title feat(sdk): SDK changes feat(sdk): deprecata batcher_url parameter Dec 5, 2024
@MarcosNicolau MarcosNicolau changed the title feat(sdk): deprecata batcher_url parameter feat(sdk): deprecate batcher_url parameter Dec 5, 2024
@MarcosNicolau MarcosNicolau requested a review from uri-99 December 5, 2024 19:32
Copy link
Contributor

@uri-99 uri-99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing remove batcher-url from rust task sender.

@MarcosNicolau MarcosNicolau requested a review from uri-99 December 5, 2024 20:58
@uri-99 uri-99 changed the title feat(sdk): deprecate batcher_url parameter feat(sdk): some SDK improvements Jan 2, 2025
Copy link
Contributor

@JulianVentura JulianVentura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected!
There are some docs conflicts

Comment on lines +472 to +475
if private_key.is_some() {
error!("Conflicting inputs detected: Both a keystore and a private key were provided. Provide only one option to proceed.");
return Ok(());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@MauroToscano MauroToscano left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR should be splitted in different PRs. Also, default network params is fine, but we should have a way to change the batcher url manually, let's re review this over the following days

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 this pull request may close these issues.

8 participants