Skip to content

Add doc/ folder, split up readme and add more usage documentation #55

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

Merged
merged 2 commits into from
Jul 20, 2021

Conversation

ryanofsky
Copy link
Collaborator

No description provided.

Move sections to new doc/design.md, doc/install.md, doc/usage.md files.

Also add link to issue tracker.
@ryanofsky ryanofsky merged commit 8abd3da into bitcoin-core:master Jul 20, 2021
@ryanofsky
Copy link
Collaborator Author

Updated comment where this was suggested bitcoin/bitcoin#19160 (comment)

@ariard
Copy link

ariard commented Jul 21, 2021

Post-merge ACK 1a3dc8d, thanks for taking the suggestion!

One of the hurdle I've met is how to pass interface pointers cleanly across processes to enable bidirectional requests. IIRC, this is illustrated in example/ though it could be nice to highlight in usage as from my experience it's one of the most interesting feature of capnproto :)

@ryanofsky
Copy link
Collaborator Author

One of the hurdle I've met is how to pass interface pointers cleanly across processes to enable bidirectional requests. IIRC, this is illustrated in example/ though it could be nice to highlight in usage as from my experience it's one of the most interesting feature of capnproto :)

Thanks this is good to know, and I'll make a note to write a description.

I will say a complication of passing interface pointers is ownership semantics. The thing that works best is to use std::unique_ptr<Interface> return values and arguments. std::shared_ptr<Interface> works too but you can have trouble with leaks and circular references if you're not careful. Interface& and Interface* return values and arguments which are used by some bitcoin interfaces are messy and painful and I'm trying to gradually get rid of them (CScheduler and RPCTimer stuff is really messy). Would definitely recommend using unique_ptr wherever possible in all new methods. Looking briefly at src/interfaces/ in https://github.com/ariard/bitcoin/commits/2021-07-altnet-lightning I didn't actually see new methods passing interfaces around except Init::makeXXX methods, but avoiding Interface& or Interface* arguments could be a good tip for the future

@ariard
Copy link

ariard commented Jul 23, 2021

Sorry the code is a mess, I think here is an example of passing interface pointers : https://github.com/ariard/bitcoin/blob/76c82fc7cbba554fbcfd2f9dfff02c338e75f051/src/interfaces/init.h#L40

Where a pointer on the Validation interface is passed from the bitcoin-node to the bitcoin-altnet process. The Validation interface is defined here : https://github.com/ariard/bitcoin/blob/2021-07-altnet-lightning/src/interfaces/validation.h. I think it's interface pointers but I might on another feature, just using the wrong name?

And yeah really true on the ownership semantics, I did end up by using std::unique_ptr<Interface> everywhere though only after trying and failing the other alternatives...

@ryanofsky
Copy link
Collaborator Author

And yeah really true on the ownership semantics, I did end up by using std::unique_ptr<Interface> everywhere though only after trying and failing the other alternatives...

Yes, I think using unique_ptr is a good thing. It ensures that all the objects shared across processes have a clear owner. It also encourages objects which are shared across processes to be lightweight wrappers, so the cross-process API that is exposed is separate from the actual implementation of features that are being provided.

@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jun 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants