-
-
Notifications
You must be signed in to change notification settings - Fork 146
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
Inbound routing fees #2354
Inbound routing fees #2354
Conversation
e6d37b7
to
9d46600
Compare
8f24495
to
b6b0c99
Compare
…inbound fees input
9f98b0c
to
69bbd25
Compare
models/ChannelInfo.ts
Outdated
@@ -6,6 +6,8 @@ interface RoutingPolicy { | |||
min_htlc: string; | |||
fee_base_msat: string; | |||
fee_rate_milli_msat: string; | |||
inbound_fee_base_msat: string; | |||
inbound_fee_rate_milli_msat: string; |
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.
let's add ?
to these two new fields as older nodes may not have them
feeRateInbound={`${ | ||
Number(policy.inbound_fee_rate_milli_msat) / | ||
10000 | ||
}`} |
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.
for these two, let's check to see if they exist or return null
- otherwise there will be cases here where it returns NaN
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.
would it be better to return 0
instead of null
?
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.
No
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.
undefined
would be best
components/FeeBreakdown.tsx
Outdated
) / 10000 | ||
}%`} | ||
sensitive | ||
/> |
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.
these two should only be displayed if supportInboundFees
69bbd25
to
7d48ef4
Compare
9aab0d7
to
e029ec1
Compare
components/SetFeesForm.tsx
Outdated
@@ -24,6 +24,8 @@ interface SetFeesFormProps { | |||
SettingsStore?: SettingsStore; | |||
baseFee?: string; | |||
feeRate?: string; | |||
baseFeeInbound: string; | |||
feeRateInbound: string; |
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.
add ?
to these two too
base_fee_msat: base_fee_msat_inbound, | ||
fee_rate_ppm: `${Number(fee_rate_inbound) * 10000}` | ||
} | ||
}), |
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.
if you're gonna go with this approach, be sure to update it in the LNC backend file as well - otherwise you can do something similar in the store
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.
did the changes!
…i only for the nodes that support it.
6d53dfa
to
67d1cd9
Compare
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.
uTACK
Description
Relates to issue: ZEUS-2185
We want to implement the ability to set the inbound routing fees
This pull request is categorized as a:
Checklist
yarn run tsc
and made sure my code compiles correctlyyarn run lint
and made sure my code didn’t contain any problematic patternsyarn run prettier
and made sure my code is formatted correctlyyarn run test
and made sure all of the tests passTesting
If you modified or added a utility file, did you add new unit tests?
I have tested this PR on the following platforms (please specify OS version and phone model/VM):
I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):
Locales
Third Party Dependencies and Packages
yarn
after this PR is merged inpackage.json
andyarn.lock
have been properly updatedOther: