Libquic integration #83
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add debian oldstable (bullseye) - Add debian testing - Use oxen repo for bionic
Remove `(amd64)` from various job names as its a sane default to assume if not otherwise noted.
For shared builds we can use it rather than needing to rebuild ngtcp2.
bionic can't cope with the C++20 that we use. Win (x86) build is broken for unknown reasons, so comment it out for now as it isn't that important.
Catch2 gives more informative diagnostics when using CHECK(thing) or CHECK_FALSE(thing) for direct boolean values.
Updated the network functions to be on a Network class Updated the logic to store the quic::Network and quic::Endpoint instances Added logic to store the current paths on the Network instance Added a shared_ptr to the quic::connection_interface against it's associated path
jagerman
reviewed
Apr 16, 2024
include/session/network.hpp
Outdated
Comment on lines
35
to
40
| const service_node target; | ||
| const std::string endpoint; | ||
| const std::optional<ustring> body; | ||
| const std::optional<std::vector<service_node>> swarm; | ||
| const std::optional<onion_path> path; | ||
| const bool is_retry; |
There was a problem hiding this comment.
I don't think these members should be const: if you already have a const request_info then all of its members are (to you) const already, and if you have a *non-*const on hand then everything being const just makes it more cumbersome to work with.
There was a problem hiding this comment.
Okay I read further and see what's going on here: I think this struct is meant to be a container of references, rather than values, in which case it should be const whatever& member.
But I think this won't be sufficient because of other changes along the way, and so I think we need to rework this a little.
There was a problem hiding this comment.
But I think this won't be sufficient because of other changes along the way, and so I think we need to rework this a little.
To elaborate: I left a comment here about not using a promise/future, and I think to make that work we'll need to make a copy of this object.
There was a problem hiding this comment.
I've removed the const from these - now that the snode pool is internal it might be easier to see how this is used (it's essentially only used to allow for the retry request in the case of receiving a 421)
- Fixes compilation failure when libquic comes from a system library, where we wouldn't get oxen::logging transitively via oxen-libquic. - Fix `onionreq` target not explicitly depending on oxen::logging - Remove oxen logging include from headers so that dependent code (e.g. an external cmake project depending on libsession-util) doesn't have to have it to include libsession headers. - set OXEN_LOGGING_SOURCE_ROOT to strip the source directory out of logged source file paths.
This build is identical to the Static Windows x64 further down.
The Network object now manages an internal service node pool cache as well as the onion request paths needed for sending requests. When calling `send_onion_request` it will check the snode pool (populating it if needed) and check for possible onion request paths (creating them if needed) before sending the request - these pool/path population approaches are blocking so the caller can trigger multiple requests and doesn't need to worry about multiple requests being triggered to populate the cache. The Network object also has logic to persist and load this cache from disk (when initialised with a `cache_path`) in order to improve launch times after retrieving an initial cache. Additionally the Network object exposes a couple of hooks so the client can register for updates to the onion request paths or the network connection status.
…sing-c-func Added a missing C func to mark a user as invited (no longer kicked)
• Split the `set_invited` function into `set_invite_sent` and `set_invite_failed` to be clearer and more consistent with the promotion functions
• Renamed the `INVITE_{status}` constants to be `STATUS_{status}` as they are used for both invite and promotion statuses
• Renamed some UserGroups functions to be snake case
Co-authored-by: Audric Ackermann <[email protected]>
…tion-standardisation Split set_invited func, renamed some constants, renamed some functions
Returning a bt_dict is clunky and slow, and increasingly we're moving away from bt_dict everywhere else in the codebase in favour of producer/consumer, so do that for extra_data too.
• Moved the member status function to the members config (allows us to avoid odd code gymnastics) • Removed the changes to the member type • Fixed a couple of CI errors
load_extra_data currently doesn't work as intended because it gets invoked from the ConfigBase base class constructor, but this constructor runs before the derived class (Members) has been constructed, and so the virtual method doesn't find a derived instance and so always goes to the do-nothing base class implementation. This solves it by moving to actual dump+key setup into an `init()` method that allows subclasses to do a two-step initialization where they default-construct the ConfigBase, and then manually call `init()` with the dump data and keys from within their own constructor, where the load_extra_data() call will work properly.
use single letter for keys dump() & set pending_ send_state
…ate' into libquic # Conflicts: # include/session/config/base.hpp # tests/test_config_userprofile.cpp
…cal_members_sending_state Local "Sending" member invite & promotion status logic
feat: allow to mark a member invite state as not sent
…ssing-c-api Added C API for new `set_invite_not_sent` function
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.