Skip to content

ChainInterface is easy to misuse #80

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.

Sign up for GitHub

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

Jump to bottom
Closed
TheBlueMatt opened this issue Jul 23, 2018 · 2 comments
Closed

ChainInterface is easy to misuse #80

TheBlueMatt opened this issue Jul 23, 2018 · 2 comments
Milestone
0.3

Comments

@TheBlueMatt
Copy link
Collaborator

In writing https://github.com/TheBlueMatt/rust-lightning-bitcoinrpc I almost misused ChainWatchInterface, almost calling ChannelManager::block_connected and PeerHandler::block_connected instead of only calling ChainWatchInterfaceUtil::block_connected (which will call the others for you).

I'm not actually sure what a better interface looks like. I played around a bit with making some things pub(crate) in chaininterface but couldn't come up with anything I actually liked, sadly.
@TheBlueMatt TheBlueMatt added this to the 0.2 milestone Sep 19, 2018
@TheBlueMatt TheBlueMatt mentioned this issue Sep 29, 2018
Closed
@TheBlueMatt TheBlueMatt mentioned this issue Nov 1, 2018
Closed
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Dec 11, 2018
@TheBlueMatt
Move monitor-generated HTLC event handling to manager event-getters
f8fedb2 
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH lightningdevkit#80.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Dec 11, 2018
@TheBlueMatt
Move monitor-generated HTLC event handling to manager event-getters
06c46ec 
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH lightningdevkit#80.

This affects full_stack_target only on accident cause the demo test
didn't continue onwards with another block connection.
@TheBlueMatt TheBlueMatt mentioned this issue Dec 11, 2018
Merged
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Dec 11, 2018
@TheBlueMatt
Move monitor-generated HTLC event handling to manager event-getters
753e960 
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH lightningdevkit#80.

This affects full_stack_target only on accident cause the demo test
didn't continue onwards with another block connection.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Dec 12, 2018
@TheBlueMatt
Move monitor-generated HTLC event handling to manager event-getters
b21489b 
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH lightningdevkit#80.

This affects full_stack_target only on accident cause the demo test
didn't continue onwards with another block connection.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Dec 13, 2018
@TheBlueMatt
Move monitor-generated HTLC event handling to manager event-getters
ffc2544 
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH lightningdevkit#80.

This affects full_stack_target only on accident cause the demo test
didn't continue onwards with another block connection.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Dec 13, 2018
@TheBlueMatt
Move monitor-generated HTLC event handling to manager event-getters
b8fe0f3 
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH lightningdevkit#80.

This affects full_stack_target only on accident cause the demo test
didn't continue onwards with another block connection.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Dec 13, 2018
@TheBlueMatt
Move monitor-generated HTLC event handling to manager event-getters
41ddb84 
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH lightningdevkit#80.

This affects full_stack_target only on accident cause the demo test
didn't continue onwards with another block connection.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Dec 14, 2018
@TheBlueMatt
Move monitor-generated HTLC event handling to manager event-getters
221bfa6 
This is somewhat awkward, but prevents a slew of duplicate events.
Really this should probably be more explicit, but would be easy to
move that along with a slew of block_connected-event-processing
refactors, see-also GH lightningdevkit#80.

This affects full_stack_target only on accident cause the demo test
didn't continue onwards with another block connection.
@dongcarl
Copy link
Contributor

I played around a bit with making some things pub(crate) in chaininterface but couldn't come up with anything I actually liked, sadly.

What was insufficient with pub(crate)?

@TheBlueMatt TheBlueMatt mentioned this issue Nov 21, 2019
Merged
@TheBlueMatt
Copy link
Collaborator Author

We don't have a ChainWatchInterfaceUtil anymore, and this interface is much much harder to mis-use now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.3
Development

No branches or pull requests
2 participants
@TheBlueMatt @dongcarl