Integrate resource manager into ChannelManager#4468
Closed
elnosh wants to merge 13 commits into
Closed
Conversation
Implements a decaying average over a rolling window. It will be used in upcoming commits by the resource manager to track reputation and revenue of channels.
The RevenueAverage implemented here will be used in upcoming commits to track the incoming revenue that channels have generated through HTLC forwards.
|
👋 Hi! I see this is a draft PR. |
Resources available in the channel will be divided into general, congestion and protected resources. Here we implement the general bucket with basic denial of service protections. Co-authored-by: Carla Kirk-Cohen <kirkcohenc@gmail.com>
Resources available in the channel will be divided into general, congestion and protected resources. Here we implement the bucket resources that will be used for congestion and protected.
The Channel struct introduced here has the core information that will be used by the resource manager to make forwarding decisions on HTLCs: - Reputation that this channel has accrued as an outgoing link in HTLC forwards. - Revenue (forwarding fees) that the channel has earned us as an incoming link. - Pending HTLCs this channel is currently holding as an outgoing link. - Bucket resources that are currently in use in general, congestion and protected.
Trait that will be used by the `ChannelManager` to mitigate slow jamming. Core responsibility will be to track resource usage to evaluate HTLC forwarding decisions.
Introduces the DefaultResourceManager struct. The core of methods that will be used to inform the HTLC forward decisions are add/resolve_htlc. - add_htlc: Based on resource availability and reputation, it evaluates whehther to forward or fail the HTLC. - resolve_htlc: Releases the bucket resources used from a HTLC previously added and updates the channel's reputation based on HTLC fees and resolution times.
Adds write and read implementations to persist the DefaultResourceManager.
Implements a wrapper around the DefaultResourceManager. The main purpose to silently handle an HTLC not found error. In read-only mode, the ChannelManager will only log the forwarding outcome suggestions from the resource manager but not fail HTLCs if instructed to do so. Since HTLCs suggested to be failed are not stored by the DefaultResourceManager, a subsequent call from the ChannelManager to resolve this HTLC would return an error because it does not know about it. In this case, the error is discarded.
- max_accepted_htlcs and max_htlc_value_in_flight_msat values in the channel are required when adding new channels in the resource manager. - get_outbound_htlcs helper will be used during restart to replay pending HTLCs.
Integrates the ReadOnlyResourceManager into the ChannelManager. This can be enabled by the enable_resource_manager field added in the UserConfig. The main 4 call sites are when adding/removing channels and add/resolve forwarded HTLCs. The add_htlc call into the resource manager is done before the queue_add_htlc on the outgoing channel. In read-only mode, it will only log the forwarding outcome suggestion from the resource manager and use the accountable signal suggested. Note that it only works in std given that we need access to the time.
elnosh
force-pushed
the
resource-mgr-readonly-integration
branch
from
db762d0 to
670c904
Compare
|
Hi @elnosh, Thanks for your contributions to After too many struggles with bugs, outages, contributor bans, and, finally, a multi-week CI ban, the rust-lightning project is moving off of GitHub for day-to-day development. You can still file issues and access the git tree here, but PRs will now take place exclusively at https://git.rust-bitcoin.org/. As such, this PR has been migrated to https://git.rust-bitcoin.org/lightningdevkit/rust-lightning/pulls/4468 If you log in using GitHub (or otherwise link your GitHub account from https://git.rust-bitcoin.org/user/settings/security), ownership of your PRs, issues, and comments will automatically transfer. To push updates to this PR, you'll need to use |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Builds on top of #4409
It adds a
ReadOnlyResourceManagerwrapper that is integrated into theChannelManager. In this iteration, the forwarding outcome would be logged but if instructed to fail an HTLC, it won't do so.A couple things to mention:
ResourceManagertrait. Here, it is not actually using it and instead just doing it asOption<ReadOnlyResourceManager>to avoid doingdyn ResourceManager. This is because it is intended to just be enabled by a user config and not actually expecting a type from the user when starting the channel manager. So perhaps that commit introducing the trait could be removed.