WIP: introduce wrapper package for C# FFI. #569
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joemphilips
force-pushed
the
ffi_build_scripts
branch
from
eb260ae to
22908a6
Compare
|
This PR passes test only in nightly, because I am using Instead, I will delay to make it pass in |
joemphilips
force-pushed
the
ffi_build_scripts
branch
2 times, most recently
from
b84dfd5 to
afb6c7c
Compare
|
Closing in favor of #618 |
|
Hey @joemphilips . I hope you didn't take #618 the wrong way - I believe @arik-so had mentioned that he chatted with you a bit and indicated that a hopefully more language-agnostic bindings system was coming, at least to expose very low-level stuff so that other languages can be based on that instead of rust directy. That said, #618 is super, super low-level bindings, its really not very friendly to work with directly, and is intended as a base to build other language bindings off of, not use. Its kinda up to you how to continue here, though - while I don't think I/the full-time team want to maintain C#-specific stuff yet, if you want to, we can still move forward with your work here as-is, its just more up to "the community" (ie you or others using it) to maintain at least in the immediate future. The other option would be to redo wrappers based on the lower-level C wrappers in #618. Either way, C# bindings would be nice. |
|
Ok, I will reopen and leave it here just for the sake of transparency.
And make a new PR at some point. |
joemphilips
force-pushed
the
ffi_build_scripts
branch
7 times, most recently
from
19addfd to
5afde89
Compare
joemphilips
force-pushed
the
ffi_build_scripts
branch
3 times, most recently
from
b0e4e44 to
32687d9
Compare
joemphilips
force-pushed
the
ffi_build_scripts
branch
2 times, most recently
from
2ff2ab6 to
f2b53e9
Compare
... for ChannelError and APIMisuseError Before this commit, When rl returns error, we don't know The actual parameter which caused the error. By returning parameterised `String` instead of predefined `&'static str`, We can give a caller improved error message.
TestLogger now has two additional methods 1. `assert_log_contains` which checks the logged messsage has entry which includes the specified string as a substring. 2. `aasert_log_regex` mostly same with above but it is more flexible that caller specifies regex which has to be satisfied instead of just a substring.
* Downgrade the `regex` crate version to the latest which works fine in rustc 1.22.0 * Be explicit about how to match against reference since old rustc is not smart enough to handle implicit reference coercion. * use separate `find` and `map` instead of `find_map` . Since it is not supported in old rust.
For when other peer has sent a invalid `announcement_signatures`
WIP: expose ChannelManager from bindings crate to ffi update bindings udpate bindings to not expose result type as FFI update prepare FFIResult finish tests for FFIResult prepare thread_handle prepare is_null prepare all FFI handler type avoid all compiler error by using ARc separate adaptor types into adaptors folder add BufferToSmall error case to FFIResult kind Add new entrypoints for fetching and testing last result for FFI add entrypoint for testing remove input argument from test ffi update update update update update amend update udpate broadcaster_wrapper ffi interface update cleanup: rename variables and remove unused deps remove unused cbindgen.toml delete outdated readme message do not specify repr(C) to Record type Add `UnwandSafe` and `RefUnwindSafe` trait bound * `Logger` and `ChainWatchInterface` were trait which has to cross ffi boundary. for a foreign language to catch a panic caused in `rust-lightning` these two traits must be `UnwindSafe` so explicitly add. Fix miscs * Use usize everywhere for len * Use `OutPoint` from lightning instead of bitcoin udpate comment start writing ffi interface for testing logger fix bug that FFITransaction points to garbage location Remove unused `&Self` parameter from FFI delegate. update add entrypoint for releasing channel manager remove unused test function fix bug in FFIChainWatchInterface Constructor Revert "remove unused test function" This reverts commit 4a1cccb. add new entrypoint send_payment fix bug in broadcaster stop receiving SafeHandle twice take everything as delegate in constructor * Since otherwise it sometimes crashes by GC in C# side. cleanup stop using debug_assertions as a feature remove some unused imports and implement Error trait for APIError Simplify FFIRoute * When passing struct through FFI boundary, if the object has nested array it will cause headache for preventing it from GCed. So we just pass binary serialized object and (de)serialize after passing. In this case, `FFIRoute`. * Prepare Debug trait for Route object. add entrpoint for getting pending events from chanman update: FFI PeerManager create/dispose methods add deep_copy method to array_struct update remove unnecessary dirty tests update: add process_event function for FFI remove repr(C) from lightning pcakge fix build error in lightning update ffi for BasicMPP fix some compiler warning return Bool from read_event Simplify ChainWatchInterface * delegate some `FFIChainWatchInteface`'s work to `ChainWatchInterfaceUtil` * remove now-useless ChannelMonitor and its related functions, objects refactor create_channel_manager to be more generic fix bug in deep_copy return internal_error directly when converting FFI pub/secret key stop using wacky deep_copy method return act_one when calling new_outbound from C# stop using FFIBytes for return value of new_outbound simplify array_struct by not using generics expose PeerManager::get_peer_node_ids to ffi fix build error after rebasing to master give channel maanger from outside world when initializing PeerManager add many ffi entrypoints for ChannelManager
impl Readable/Writable to Vec<ChannelDetails> * use fixed length array when possible remove unused low-level structs do not drop PeerManager::message_handler in PeerManager destructor
joemphilips
force-pushed
the
ffi_build_scripts
branch
from
97ac07c to
082d03e
Compare
Which is a facade for `ChannelManager::send_payment` Since that PeerManager holds an information about current network graph, we need this entrypoint to not let user caluculate the path by themselves. It also deletes some useless imports and unused function `static_assert` from original `rust-csharp-ffi`
* Because we want to pass to `get_route` function when calculating the route.
* So that we can handle it by `?` operator in ffi functions
* Add `get_network_graph` function to `PeerManager` * Add `get_route_and_send_payment` to `ChannelManager` These two changes makes those two managers loosely coupled. Before this sometimes `send_payment` cause crash because the `ChannelManager` has been moved by GC. * Add `tell_block_connected_after_resume` endpoint to `ManyChannelMonitor` ffi. So that after deserialization we can add new blocks. * Return pairs of latest block hashes when deserializing `ManyChannelMonitor` So that the api consumer can query new blocks after deserialization.
* Since it uselessly complicates the ffi function by forcing us to specify `unsafe_block` every time we call it.
* For object which comes from C# side, if we take a reference as `Arc`, then rust will drop the inner value when the function exits. leaving the pointer dangling.
`ChannelManager::broadcast_node_announcement`
Before this commit, `fn get_inbound_outbound_available_balance_msat` always returns 0. It is because using `cmp::min` instead of `cmp::max` .
|
This is pretty sorely out-of-date, dunno that anyone has time to adopt this now, and it would probably need to be done almost from scratch at this point, sadly. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Following the strategy demonstrated in rust-csharp-ffi, I think it is possible to create a high level wrapper for
lightningcrate.This is still a PoC phase, it is not finished at all. This PR is just for getting a feedback for my general approach.
Right now it only has a feature to create and destroy
ChannelMonitor.It might be easy to just wrap a low-level feature such as
PeerChannelEncryptor, but I'm not very interested to this. Since it is not that hard to port and maintain (I've already did once)First I want to make sure that running it as an asp.net core server is a sane approach.
This is my first time writing rust seriously, so any feedback is welcome.
Future plan is to
ChannelManagerandPeerManagerin a similar way withChannelMonitorC# side is here