Skip to content

IPR request types v8 #5498

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

Merged
merged 20 commits into from
Feb 27, 2025
Merged

IPR request types v8 #5498

merged 20 commits into from
Feb 27, 2025

Conversation

octol
Copy link
Contributor

@octol octol commented Feb 21, 2025

Bump IPR request/response types to v8


This change is Reviewable

Copy link

vercel bot commented Feb 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
docs-nextra ⬜️ Ignored (Inspect) Visit Preview Feb 27, 2025 1:59pm
nym-next-explorer ⬜️ Ignored (Inspect) Visit Preview Feb 27, 2025 1:59pm

@octol octol marked this pull request as ready for review February 26, 2025 08:34
@octol octol requested review from jstuczyn and neacsu February 26, 2025 08:34
Copy link
Contributor

@neacsu neacsu left a comment

Choose a reason for hiding this comment

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

Reviewed 31 of 31 files at r1, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions (waiting on @octol)


service-providers/ip-packet-router/src/clients/connected_client_handler.rs line 33 at r1 (raw file):

pub(crate) struct ConnectedClientHandler {
    // The address of the client that this handler is connected to
    // nym_address: Recipient,

This looks like it could be removed


service-providers/ip-packet-router/src/clients/connected_client_handler.rs line 81 at r1 (raw file):

        let connected_client_handler = ConnectedClientHandler {
            // nym_address: reply_to,
            // reply_to_tag,

This too


sdk/rust/nym-sdk/src/mixnet/native_client.rs line 284 at r1 (raw file):

                    // I *think* this happens for SURBs, but I'm not 100% sure. Nonetheless it's
                    // beneign, but let's log it here anyway as a reminder
                    debug!("the reconstructed messages vector is empty - please let the developers know if you see this message");

Reading the message, this doesn't seem like a debug message. Maybe the content also needs to be updated?

Copy link
Contributor Author

@octol octol left a comment

Choose a reason for hiding this comment

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

Reviewable status: 8 of 45 files reviewed, 8 unresolved discussions (waiting on @jstuczyn and @neacsu)


sdk/rust/nym-sdk/src/mixnet/native_client.rs line 284 at r1 (raw file):

Previously, neacsu (Bogdan-Ștefan Neacşu) wrote…

Reading the message, this doesn't seem like a debug message. Maybe the content also needs to be updated?

Updated the message


service-providers/ip-packet-router/src/clients/connected_client_handler.rs line 33 at r1 (raw file):

Previously, neacsu (Bogdan-Ștefan Neacşu) wrote…

This looks like it could be removed

Done.


service-providers/ip-packet-router/src/clients/connected_client_handler.rs line 81 at r1 (raw file):

Previously, neacsu (Bogdan-Ștefan Neacşu) wrote…

This too

Done.

@benedettadavico benedettadavico added this to the Dorina milestone Feb 27, 2025
@octol octol merged commit 856dbfe into release/2025.4-dorina Feb 27, 2025
18 of 19 checks passed
@octol octol deleted the jon/reply-tests branch February 27, 2025 14:21
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.

4 participants