-
Notifications
You must be signed in to change notification settings - Fork 110
cmd/swarm, swarm, swap, api: configurable payment/disconnect thresholds #1729
Changes from 10 commits
2050903
83b9c3f
82b100e
6adc554
578a2fc
60d5a56
aa69db6
4460d9a
db83c9e
df154dc
bb85b52
c49f905
ebd8314
2cf5475
b6dea35
9bbbbbc
627fb77
a3b5137
70092e6
33f0bc3
b0d1e54
db12380
5d7e68a
ff20676
a1e0845
068e1dd
f174f0d
5227f7a
9139dcc
faa6435
0368a99
d652aa6
782b83d
4d5a5ee
63b04ab
c0cc688
2765e9f
0f753f2
c5edfc2
acb3d89
b8d9c0b
e6de6c8
d0a7bc4
bf928a2
49f0006
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -65,6 +65,16 @@ var ( | |
| Usage: "URL of the Ethereum API provider to use to settle SWAP payments", | ||
| EnvVar: SwarmEnvSwapBackendURL, | ||
| } | ||
| SwarmSwapPaymentThresholdFlag = cli.Uint64Flag{ | ||
| Name: "swap-payment-threshold", | ||
| Usage: "honey amount indebted to a peer at which you will initiate payment", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is only from one peer's point of view. I would change this to "honey amount at which a payment is triggered"
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in anything outside of the test directory, we should make the comments and variable names as if it is from the perspective of the node operator. Especially here, as this is a user-facing comment.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe, but even then, this node operator can cross the payment threshold in both ways. The threshold doesn't define if you are indebted or not, which is defined only in which direction you cross it. I would like to insist on this being changed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, this threshold does define whether a node is indebted or not, because the threshold which is set here only applies to what this particular node does. Sure, there are other payment thresholds in the network, and the node can cross these from the other direction (and get paid). but this particular threshold is only about how much the node that the user is about to boot up must be indebted to other peers until payment will be initiated.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if we would add a log Warn on the creditor node that a peer crossed the threshold and is indebted with him? Check this exact comment I made on another PR: #1754 (comment) If we did this (and if we add some other metric or anything to help nodes track that peers got indebted to them), then your comment would not apply. |
||
| EnvVar: SwarmEnvSwapPaymentThreshold, | ||
| } | ||
| SwarmSwapDisconnectThresholdFlag = cli.Uint64Flag{ | ||
| Name: "swap-disconnect-threshold", | ||
| Usage: "honey amount at debt of a peer at which you will disconnect", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And here "honey amount at which a peers disconnect"
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to keep this from the point of view of the node operator
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above |
||
| EnvVar: SwarmEnvSwapDisconnectThreshold, | ||
| } | ||
| SwarmSyncDisabledFlag = cli.BoolTFlag{ | ||
| Name: "nosync", | ||
| Usage: "Disable swarm syncing", | ||
|
|
||
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.
more explanation in comment needed
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.
What do you mean here?
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.
please comment exported constants, explain what they are
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.
Done