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

DO NOT MERGE - Async networking #67

Closed
wants to merge 35 commits into from

Conversation

LimpidCrypto
Copy link
Collaborator

@LimpidCrypto LimpidCrypto commented May 15, 2023

High Level Overview of Change

Adds async networking abilities:

  • AsyncWebsocketClient
  • AsyncJsonRpcClient

Due to the milestones complexity, a new crate em-as-net will be created. For a high level overview of the state of em-as-net see this issue:

Important to notice: At the time of writing the crate is still missing TlsConnection to establish safe connections.

The PR also contains smaller improvements like making ˋnewˋ methods public and importing the ˋToStringˋ trait in the ˋErr!ˋ macro instead of importing it in every file that uses the macro.

TODO

  • no_std support for clients here in xrpl-rust and in em-as-net
  • Establish safe connections using TlsConnection in em-as-net

Type of Change

  • New feature (non-breaking change which adds functionality)

Test Plan

A code sample was added to tests/common.rs

@LimpidCrypto LimpidCrypto added the enhancement New feature or request label May 15, 2023
@LimpidCrypto LimpidCrypto added this to the Add clients milestone May 15, 2023
@LimpidCrypto LimpidCrypto self-assigned this May 15, 2023
@LimpidCrypto LimpidCrypto linked an issue May 15, 2023 that may be closed by this pull request
@LimpidCrypto LimpidCrypto changed the title get changes from networking branch and use dev branch as base so all … DO NOT MERGE - Async networking May 15, 2023
@LimpidCrypto LimpidCrypto linked an issue May 15, 2023 that may be closed by this pull request
.gitignore Outdated Show resolved Hide resolved
src/asynch/clients/websocket_base.rs Outdated Show resolved Hide resolved
tests/common.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@LimpidCrypto
Copy link
Collaborator Author

I still have an issue with the github workflow, claiming that it can't find xrpl::asynch::clients and rand::rngs::ThreadRng:

error[E0432]: unresolved import `rand::rngs::ThreadRng`
 --> tests/common/mod.rs:4:5
  |
4 | use rand::rngs::ThreadRng;
  |     ^^^^^^^^^^^^^^^^^^^^^ no `ThreadRng` in `rngs`
error[E0432]: unresolved import `xrpl::asynch::clients`
 --> tests/common/mod.rs:6:19
  |
6 | use xrpl::asynch::clients::{
  |                   ^^^^^^^ could not find `clients` in `asynch`
error[E0432]: unresolved import `xrpl::asynch::clients`
 --> tests/integration/clients/mod.rs:6:23
  |
6 |     use xrpl::asynch::clients::{ReadResult, WebsocketClose, WebsocketIo};
  |                       ^^^^^^^ could not find `clients` in `asynch`

@@ -17,6 +17,7 @@ pub mod ledger;
pub mod ledger_closed;
pub mod ledger_current;
pub mod ledger_data;
#[allow(clippy::result_large_err)]
Copy link
Owner

Choose a reason for hiding this comment

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

Do we want to do this?

@@ -9,6 +9,7 @@

pub mod exceptions;
#[cfg(feature = "ledger")]
#[allow(clippy::too_many_arguments)]
Copy link
Owner

Choose a reason for hiding this comment

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

Should we allow these exceptions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I allowed too_many_arguments because the new methods have a lot of parameters. What would be your suggestion to resolve the clippy warning?

@LimpidCrypto LimpidCrypto mentioned this pull request Aug 14, 2023
2 tasks
@LimpidCrypto
Copy link
Collaborator Author

Closed in favor of #70

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add clients private ˋnewˋ methods
2 participants