Skip to content
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

Add routing view #22

Merged
merged 3 commits into from
Jun 9, 2021
Merged

Add routing view #22

merged 3 commits into from
Jun 9, 2021

Conversation

mmilata
Copy link
Contributor

@mmilata mmilata commented May 9, 2021

Adds ROUTING view which shows log of HTLC forwards, sends & receives. It is useful for viewing failed forwards which are (AFAIK) not persisted in lnd but can be obtained through streaming api. Not quite sure what the best column order & column colors are:smiley:

Needs #21.

ss

@mmilata mmilata force-pushed the routing-view branch 2 times, most recently from 3ed1101 to 074cf08 Compare May 31, 2021 09:46
@mmilata mmilata marked this pull request as ready for review May 31, 2021 10:20
Copy link
Owner

@edouardparis edouardparis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement 👏
Please, give in a comment a link to a lnurl-pay or web page to generate an invoice for tip

"DIR", # event type: send, receive, forward
"STATUS", # one of: active, settled, failed, linkfail
"IN CHANNEL", # channel id of the incomming channel
"IN ALIAS", # incoming channel node alias
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it difficult to understand from colum header the limits of each column
maybe It should be better to have underscore: IN_ALIAS in order to have space as a column delimiter only

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README.md should be modified with this new columns and a disclamer about the non persistence of the routing view

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed and added section to README.md. What about LAST UPDATE though?

@@ -425,6 +480,10 @@ func New(c *config.Network, logger logging.Logger) (*Backend, error) {
logger: logger.With(logging.String("name", c.Name)),
}

if c.PoolCapacity < lndMinPoolCapacity {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah my pool has bug, technically we want the pool to create a new connection if there is not enough conn.
Your fix can be added temporally the time to fix it

}

func (c *Routing) Speed() (int, int, int, int) {
current := c.currentColumnIndex()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added fix to bc8aabe

@mmilata
Copy link
Contributor Author

mmilata commented Jun 8, 2021

Thanks for the tip! I currently cannot accept as I don't want this identity publicly linked to my node, but feel free to tip nix-bitcoin instead:)

Comment on lines 26 to 29
"IN CHANNEL",
"IN ALIAS",
"OUT CHANNEL",
"OUT ALIAS",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry it is missing _
Columns are not displayed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, fixed in f0dedd5.

Ignore lower values, lntop won't start with pool_capacity <= 3 due to
router rpc handle.
@edouardparis edouardparis merged commit c923d7a into edouardparis:master Jun 9, 2021
@edouardparis
Copy link
Owner

Thank you for your contribution !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants