ldk wrapper - #136
Conversation
carlaKC
left a comment
carlaKC
left a comment
There was a problem hiding this comment.
Nice, just a few nits then this is ready to go.
Were the main changed needed for the LDK branch just utilities to get snapshots etc?
Also did you run any of the existing attacks with this, interested to know if there are any behavior changes.
| fn params_to_config(params: &ForwardManagerParams) -> ResourceManagerConfig { | ||
| ResourceManagerConfig { | ||
| general_allocation_pct: params.general_slot_portion, | ||
| congestion_allocation_pct: params.congestion_slot_portion, | ||
| resolution_period: params.reputation_params.resolution_period, | ||
| revenue_window: params.reputation_params.revenue_window, | ||
| reputation_multiplier: params.reputation_params.reputation_multiplier, | ||
| } | ||
| } | ||
|
|
||
| fn bucket_from_snapshot(snapshot: BucketSnapshot) -> BucketResources { | ||
| BucketResources { | ||
| slots_used: snapshot.slots_used, | ||
| slots_available: snapshot.slots_available, | ||
| liquidity_used_msat: snapshot.liquidity_used_msat, | ||
| liquidity_available_msat: snapshot.liquidity_available_msat, | ||
| } | ||
| } |
There was a problem hiding this comment.
couldn't do it because BucketSnapshot and ResourceManagerConfig come from ldk
| self.inner.add_channel_with_reputation( | ||
| channel_id, | ||
| max_in_flight, | ||
| MAX_ACCEPTED_HTLCS, |
There was a problem hiding this comment.
Maybe use the graph's max_htlc_count rather than a hard set default?
There was a problem hiding this comment.
should we fix separately? we use that hard default in ForwardManager too
yes, main reasons were:
|
Wraps LDK's DefaultResourceManager and implements the simulator's ReputationManager trait so that the LDK resource manager can be used in the simulator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Makes ReputationInterceptor generic over the resource manager and adds an ldk-resource-manager feature to select the LDK implementation at compile time. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
I ran slow jam attack and saw similar results. Although that does not touch general and congestion functionality as those are hard set to "jammed" |
That's the case for most of the attacks, so I think it's fine! Thanks for re-running |
2 participants
I've ran a couple simulations with it. Will do some more testing before moving out of draft.
It is using changes from this branch which mostly added added methods needed to impl the
ReputationManager.