-
Notifications
You must be signed in to change notification settings - Fork 55
Add --capacity-multiplier option to SimLN plugin for random activity #729
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
base: main
Are you sure you want to change the base?
Add --capacity-multiplier option to SimLN plugin for random activity #729
Conversation
4584c4e
to
ea0c739
Compare
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not working as expected and the test, which seems to pass, does not cover an increase in simln activity due to a multiplier being set.
The expected behavior can be produced (for testing purposes) by removing the activity from network.yaml (to force simln to generate random activity):
diff --git a/test/data/network_with_plugins/network.yaml b/test/data/network_with_plugins/network.yaml
index 6e4d64a3..98228473 100644
--- a/test/data/network_with_plugins/network.yaml
+++ b/test/data/network_with_plugins/network.yaml
@@ -69,7 +69,6 @@ plugins: # Each plugin section has a number of hooks available (preDeploy, post
helloTo: "postDeploy!"
simln: # You can have multiple plugins per hook
entrypoint: "../../../resources/plugins/simln"
- activity: '[{"source": "tank-0003-ln", "destination": "tank-0005-ln", "interval_secs": 1, "amount_msat": 2000}]'
preNode: # preNode plugins run before each node is deployed
hello:
entrypoint: "../plugins/hello"
Normal simln log:
Starting activity producer for tank-0003-ln(035f94...af68f9):
activity generator for capacity: 50000000 with multiplier 2:
26.31578947368421 payments per month (0.03654970760233918 per hour).
Starting activity producer for tank-0004-ln(0215a7...86a6f9):
activity generator for capacity: 75000000 with multiplier 2:
39.473684210526315 payments per month (0.05482456140350877 per hour).
Starting activity producer for tank-0005-ln(026c9d...73cadb):
activity generator for capacity: 25000000 with multiplier 2:
13.157894736842104 payments per month (0.01827485380116959 per hour).
... and then hard-code the multiplier value and restart:
diff --git a/resources/plugins/simln/charts/simln/templates/pod.yaml b/resources/plugins/simln/charts/simln/templates/pod.yaml
index 69790c9e..2aa8eca0 100644
--- a/resources/plugins/simln/charts/simln/templates/pod.yaml
+++ b/resources/plugins/simln/charts/simln/templates/pod.yaml
@@ -37,6 +37,7 @@ spec:
- >
cd /working;
sim-cli
+ --capacity-multiplier=10000
volumeMounts:
- name: {{ .Values.workingVolume.name }}
mountPath: {{ .Values.workingVolume.mountPath }}
Simln log with multiplier applied:
Starting activity producer for tank-0003-ln(03ea88...184e48):
activity generator for capacity: 50000000 with multiplier 10000:
131578.94736842104 payments per month (182.7485380116959 per hour).
Starting activity producer for tank-0004-ln(038fae...5d1ac3):
activity generator for capacity: 75000000 with multiplier 10000:
197368.42105263157 payments per month (274.12280701754383 per hour).
Starting activity producer for tank-0005-ln(023768...c1ad57):
activity generator for capacity: 25000000 with multiplier 10000:
65789.47368421052 payments per month (91.37426900584795 per hour).
resources/plugins/simln/plugin.py
Outdated
# Execute sim-cli with capacity multiplier if provided | ||
if capacity_multiplier is not None: | ||
# Wait for the main container to be ready | ||
time.sleep(5) # Give the container time to start | ||
# Execute the command with capacity multiplier | ||
_sh( | ||
name, | ||
"sh", | ||
("-c", f"cd /working && sim-cli --capacity-multiplier {capacity_multiplier}"), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure this is going to start a second simln process inside the container which is undesirable. There is probably a better way to pass the parameter to the primary process of the container that is all helm stuff and doesn't need any python at all (i.e. add it to spec.containers.args
in simln pod.yaml.
Or if you wanna stay in python you can probably just add the value to the sim.json that already configures the desired simln activity.
resources/plugins/simln/plugin.py
Outdated
@@ -42,6 +42,7 @@ class PluginError(Exception): | |||
|
|||
class PluginContent(Enum): | |||
ACTIVITY = "activity" | |||
CAPACITY_MULTIPLIER = "capacity_multiplier" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you meant this, because this would be the name of the field in the yaml examples?
CAPACITY_MULTIPLIER = "capacity_multiplier" | |
CAPACITY_MULTIPLIER = "capacityMultiplier" |
test/plugin_test.py
Outdated
@@ -94,6 +95,36 @@ def assert_hello_plugin(self): | |||
wait_for_pod("tank-0005-post-hello-pod") | |||
wait_for_pod("tank-0005-pre-hello-pod") | |||
|
|||
def test_capacity_multiplier_option(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try testing the multiplier from the network.yaml file instead of using the plugin's commands. Personally I think giving plugins CLI commands like launch-activity
was a mistake 🤷 At this point the plugin should already be running.
resources/plugins/simln/README.md
Outdated
activity: '[{"source": "tank-0003-ln", "destination": "tank-0005-ln", "interval_secs": 1, "amount_msat": 2000}]' | ||
```` | ||
capacity_multiplier: 2.5 # Optional: Capacity multiplier for random activity |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or is it capacityMultiplier
?
resources/plugins/simln/plugin.py
Outdated
if capacity_multiplier is not None: | ||
command += f" --set capacityMultiplier={capacity_multiplier}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what this is for, you are reading a value from the network.yaml file but then also setting it using helm? (well, reading a snake_case
value but then setting a camelCase
so its confusing...)
Thank you for the review I will work on the changes |
ea0c739
to
8dad44b
Compare
8dad44b
to
ab9aed1
Compare
This PR adds support for the
--capacity-multiplier
option to the SimLN plugin, addressing issue #721. This option allows users to scale payment amounts based on channel capacity for more realistic Lightning Network simulations.Changes Made
Core Implementation
resources/plugins/simln/plugin.py
: AddedCAPACITY_MULTIPLIER
enum and--capacity-multiplier
CLI optionresources/plugins/simln/charts/simln/values.yaml
: Added Helm chart configuration for capacity multiplierresources/plugins/simln/README.md
: Updated documentation with usage examplesTesting
test/plugin_test.py
: Updated integration test for capacity multiplier functionalityTesting
Files Changed
resources/plugins/simln/plugin.py
resources/plugins/simln/charts/simln/values.yaml
resources/plugins/simln/README.md
test/plugin_test.py
Closes #721