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.

Sign up for GitHub

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

Jump to bottom

test(wallet): add thread safety test #1417

Merged
merged 1 commit into from
May 4, 2024
Merged

test(wallet): add thread safety test #1417

merged 1 commit into from
May 4, 2024

Conversation

rustaceanrob
Copy link
Contributor

Description

Wallet auto-implements Send and Sync after removing the generic. This test is a compile time error if there are changes to Wallet in the future that make it unsafe to send between threads. See #1387 for discussion.

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

Bugfixes:

  • This pull request breaks the existing API
  • I've added tests to reproduce the issue which are now passing
  • I'm linking the issue being fixed by this PR

storopoli
storopoli approved these changes May 1, 2024
View reviewed changes
Copy link
Contributor

@storopoli storopoli left a comment

Choose a reason for hiding this comment

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

Concept ACK, these tiny but powerful tests are really a nice addition.

evanlinjin
evanlinjin approved these changes May 2, 2024
View reviewed changes
Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

ACK ac18525

Good idea, thanks for doing this.

crates/bdk/tests/wallet.rs Outdated Show resolved Hide resolved
@notmandatory notmandatory added this to the 1.0.0-alpha milestone May 2, 2024
evanlinjin
evanlinjin approved these changes May 4, 2024
View reviewed changes
Copy link
Member

@evanlinjin evanlinjin left a comment

Choose a reason for hiding this comment

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

ACK db47347

@evanlinjin evanlinjin merged commit 2703cc6 into bitcoindevkit:master May 4, 2024
12 checks passed
@rustaceanrob rustaceanrob deleted the thread-safety branch May 4, 2024 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@storopoli storopoli storopoli approved these changes

@evanlinjin evanlinjin evanlinjin approved these changes

Assignees

@rustaceanrob rustaceanrob

Labels
tests
Projects
BDK
Archived in project
Milestone
1.0.0-alpha
Development

Successfully merging this pull request may close these issues.
4 participants
@rustaceanrob @evanlinjin @storopoli @notmandatory