Skip to content

Conversation

@sputn1ck
Copy link
Member

@sputn1ck sputn1ck commented Mar 1, 2024

This PR adds the listinstantouts cli command which let's users monitor their instant out swaps.

@sputn1ck sputn1ck requested review from bhandras and hieblmi March 1, 2024 13:38
Copy link
Collaborator

@hieblmi hieblmi left a comment

Choose a reason for hiding this comment

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

Was thinking that we could provide getter methods for the exposed fields rather than making them public.

initiationHeight: initCtx.initationHeight,
outgoingChanSet: initCtx.outgoingChanSet,
cltvExpiry: initCtx.cltvExpiry,
CltvExpiry: initCtx.cltvExpiry,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternatively, to not open up the scope, we could just provide getters for these members. That would also reduce the diff.

Copy link
Member

Choose a reason for hiding this comment

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

I'd tend to agree, but given that the added use-case doesn't involve sharing state as all instantouts are fetched from the DB, we can keep the exported members too.

@sputn1ck
Copy link
Member Author

sputn1ck commented Mar 3, 2024

Was thinking that we could provide getter methods for the exposed fields rather than making them public.

func (m *Manager) ListInstantOuts just fetches from the db, so there is no way to change any runtime critical data. Also i feel we don't use getters a lot in go in general?

Copy link
Member

@bhandras bhandras left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

initiationHeight: initCtx.initationHeight,
outgoingChanSet: initCtx.outgoingChanSet,
cltvExpiry: initCtx.cltvExpiry,
CltvExpiry: initCtx.cltvExpiry,
Copy link
Member

Choose a reason for hiding this comment

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

I'd tend to agree, but given that the added use-case doesn't involve sharing state as all instantouts are fetched from the DB, we can keep the exported members too.

@sputn1ck sputn1ck requested a review from hieblmi March 4, 2024 16:33
Copy link
Collaborator

@hieblmi hieblmi left a comment

Choose a reason for hiding this comment

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

LGTM then 💾

@sputn1ck sputn1ck merged commit 5d75e99 into lightninglabs:master Mar 5, 2024
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.

3 participants