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

feat: get_deposits returns accepted and pending #1479

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Jiloc
Copy link
Collaborator

@Jiloc Jiloc commented Mar 5, 2025

Description

Closes: #1463

Changes

  • renamed the previous get_deposits -> get_deposits_with_status.
  • add a new get_deposits implementation that returns both Pending and Accepted deposits. If only one of the two calls fails, we log an error but still return the deposits fetched by the other call.

Testing Information

  • added an integration test

Checklist:

  • I have performed a self-review of my code
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@Jiloc Jiloc added the sbtc signer binary The sBTC Bootstrap Signer. label Mar 5, 2025
@Jiloc Jiloc added this to the sBTC: Withdrawal fine tuning milestone Mar 5, 2025
@Jiloc Jiloc self-assigned this Mar 5, 2025
@Jiloc Jiloc requested a review from MCJOHN974 March 5, 2025 12:45
@Jiloc Jiloc changed the title [feature] get_deposits returns accepted and pending feat: get_deposits returns accepted and pending Mar 5, 2025
Copy link
Collaborator

@matteojug matteojug left a comment

Choose a reason for hiding this comment

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

Do we need to halve/change pagination_timeout default now that potentially do twice the amount of fetching?

fn get_deposits(
&self,
) -> impl std::future::Future<Output = Result<Vec<CreateDepositRequest>, Error>> + Send;

/// Get pending deposits from Emily.
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: outdated comment

Comment on lines +753 to +755
// Check that we get all deposits
let deposits = emily_client.get_deposits().await.unwrap();
assert_eq!(deposits.len(), num_deposits);
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: can we also check that we got num_accepted deposits with an Accepted state?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sbtc signer binary The sBTC Bootstrap Signer.
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

[Feature] The signers request Pending and Accepted deposit requests from Emily
2 participants