Skip to content

Support Send trait in the SDK #2235

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
Merged
tzemanovic merged 5 commits into from
Dec 7, 2023
Merged

Support Send trait in the SDK #2235

tzemanovic merged 5 commits into from
Dec 7, 2023

Conversation

@murisi
Copy link
Collaborator

@murisi murisi commented Nov 30, 2023 •
edited
Loading

Describe your changes

Added Send trait support to the SDK. More specifically, made the following changes:

  • Conditioned the Send trait support of Io, Namada, FsShieldedUtils, NamadaImpl, NullIo, and StdIo on async-send flag
  • Implemented MaybeSync and MaybeSend traits that are either Sync and Send or neither depending on the async-send flag
  • Used these traits in order to conditionally mark type constraint clauses, especially for the associated types of the Namada trait: Io, WalletUtils, ShieldedUtils, and Io
  • Upgraded the ledger-namada-rs dependency since the previous revision used is now missing and hence causing CI to fail

See the namada-faucet branch https://github.com/heliaxdev/namada-faucet/tree/murisi/try-send-trait for a demonstration for how the Send trait is used.

Indicate on which release or other PRs this topic is based on

#2225

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

@murisi murisi requested a review from Fraccaman November 30, 2023 09:40
@murisi murisi requested review from sug0 and tzemanovic November 30, 2023 10:35
@murisi murisi changed the title Fraccaman+murisi/with async Support Send trait in the SDK Nov 30, 2023
@murisi murisi changed the title Support Send trait in the SDK Support Send trait in the SDK Nov 30, 2023
@murisi murisi marked this pull request as ready for review November 30, 2023 12:19
Fraccaman
Fraccaman previously approved these changes Nov 30, 2023
View reviewed changes
@murisi murisi mentioned this pull request Nov 30, 2023
Closed
murisi added a commit that referenced this pull request Nov 30, 2023
@murisi
Merge branch 'origin/fraccaman+murisi/with-async' (#2235)
336e9d2
@murisi murisi mentioned this pull request Nov 30, 2023
Closed
2 tasks
sug0
sug0 approved these changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@sug0 sug0 left a comment

Choose a reason for hiding this comment

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

lgtm

sdk/src/lib.rs
Comment on lines +71 to +78
#[cfg(not(feature = "async-send"))]
pub trait MaybeSync {}
#[cfg(not(feature = "async-send"))]
impl<T> MaybeSync for T where T: ?Sized {}
#[cfg(not(feature = "async-send"))]
pub trait MaybeSend {}
#[cfg(not(feature = "async-send"))]
impl<T> MaybeSend for T where T: ?Sized {}
Copy link
Collaborator

@sug0 sug0 Dec 7, 2023

Choose a reason for hiding this comment

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

nice trick

@tzemanovic tzemanovic merged commit 96f18b9 into main Dec 7, 2023
@tzemanovic tzemanovic deleted the fraccaman+murisi/with-async branch December 7, 2023 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tzemanovic tzemanovic tzemanovic approved these changes

@sug0 sug0 sug0 approved these changes

@Fraccaman Fraccaman Fraccaman left review comments

Assignees

No one assigned

Labels

SDK

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@murisi @tzemanovic @Fraccaman @sug0