Refactor ChannelConfig / MaxDustHTLCExposure
#350
Merged
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
tnull
force-pushed
the
2024-08-channel-config-refactor
branch
4 times, most recently
from
1ff6d08
to
7ba4abc
Compare
jkczyz
reviewed
Aug 27, 2024
src/types.rs
Outdated
| /// This sets a fixed limit on the total dust exposure in millisatoshis. | ||
| /// | ||
| /// Please refer to [`LdkMaxDustHTLCExposure`] for further details. | ||
| FixedLimitMsat { |
There was a problem hiding this comment.
Do we need to duplicate the units here?
There was a problem hiding this comment.
You mean I could drop the Msat part? I guess so, now added a fixup.
| /// Please refer to [`LdkMaxDustHTLCExposure`] for further details. | ||
| FeeRateMultiplier { | ||
| /// The applied fee rate multiplier. | ||
| multiplier: u64, |
There was a problem hiding this comment.
Should we include the units _sats_per_kw?
There was a problem hiding this comment.
I don't think so, a multiplier is unitless?
There was a problem hiding this comment.
Now added a fixup dropping the (in sats/KW) from the comment as IMO it's misleading (the feerate has a unit, but it doesn't matter which one, the multiplier is just a multiplier.
Previously, we chose to expose `ChannelConfig` as a Uniffi `interface`, providing accessor methods. Unfortunately this forced us to `Arc` it everywhere in the API, and also didn't allow to retrieve the currently set dust exposure limits. Here, we refactor our version of `ChannelConfig` to be a normal `struct` (Uniffi `dictionary`), and only expose the `MaxDustHTLCExposure` as an enum-`interface`.
tnull
force-pushed
the
2024-08-channel-config-refactor
branch
from
5695195
to
09d68ee
Compare
Squashed without further changes. |
jkczyz
approved these changes
Aug 28, 2024
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.
Closes #349.
Previously, we chose to expose
ChannelConfigas a Uniffiinterface, providing accessor methods. Unfortunately this forced us toArcit everywhere in the API, and also didn't allow to retrieve the currently set dust exposure limits. Here, we refactor our version ofChannelConfigto be a normalstruct(Uniffidictionary), and only expose theMaxDustHTLCExposureas an enum-interface.