f readme - tons of tweaks from jeff and val

Author: TheBlueMatt

lightning-c-bindings/README.md

@@ -25,15 +25,16 @@ type getting an `LDK` prefix to their native Rust type names.
    been mapped from equivalent Rust constructors.
 
    Note that, thanks to the is-reference flag and the pointer being NULLable, such structs
-   effectively represent both `&RustThing`, `RustThing`, and `Option<RustThing>`. Check the
-   corresponding Rust documentation for the function or struct you are using to ensure you use
-   the correct call semantics.
+   effectively represent `RustType`, `&RustType`, and `Option<RustType>`. Check the corresponding
+   Rust documentation for the function or struct you are using to ensure you use the correct call
+   semantics. The passed struct must match the call semantics or an assertion failure or NULL
+   pointer dereference may occur.
 
    For example, this is the mapping of ChannelManager.
-   ```
+   ```c
    typedef struct MUST_USE_STRUCT LDKChannelManager {
       /** ... */
-      LDKlnChannelManager *inner;
+      LDKnativeChannelManager *inner;
       bool _underlying_ref;
    } LDKChannelManager;
    ```
@@ -48,16 +49,16 @@ type getting an `LDK` prefix to their native Rust type names.
    This should remind you of a C++ vtable, only written out by hand and with the class only
    containing a pointer, instead of the regular class data.
 
-   Each trait additionally contains a `free` and `clone` function pointer, which may be NULL. The
+   Each trait additionally contains `free` and `clone` function pointers, which may be NULL. The
    `free` function is passed the void pointer when the object is `Drop`ed in Rust. The `clone`
    function is passed the void pointer when the object is `Clone`ed in Rust, returning a new void
    pointer for the new object. If the `free` pointer is NULL, you will not receive any notification
-   when the trait is no longer needed. If the `clone` pointer is NULL, we assume that the trait
-   object may be `memcpy()`'d to create copies. Note that if you release resources with `free`
+   when the trait object is no longer needed. If the `clone` pointer is NULL, we assume that the
+   trait object may be `memcpy()`'d to create copies. Note that if you release resources with `free`
    without implementing `clone`, you will likely end up with use-after-free bugs.
 
    For example, `LDKSocketDescriptor` is mapped as follows:
-   ```
+   ```c
    typedef struct LDKSocketDescriptor {
       void *this_arg;
       /** ... */
@@ -71,21 +72,26 @@ type getting an `LDK` prefix to their native Rust type names.
    } LDKSocketDescriptor;
    ```
 
- * Rust structs that implement a trait result in the generation of an `X_as_Y` function which allows
-   you to use the native Rust object in place of the trait. Such generated objects are only valid as
-   long as the original Rust native object is owned by a C-wrapped struct, and has not been `free`'d
-   or moved as a part of a Rust function call.
+ * Rust structs that implement a trait result in the generation of an `X_as_Y` function, which takes
+   a C struct wrapping the Rust native object and returns a generated trait object. Such generated
+   objects are only valid as long as the original Rust native object has not been `free`'d or moved
+   as a part of a Rust function call (ie continues to be owned by the C struct). For example, to use
+   an `LDKInMemoryChannelKeys` as a `ChannelKeys`, `InMemoryChannelKeys_as_ChannelKeys` is exposed:
 
- * Rust "unitary" enums are mapped simply as an equivalent C enum, however some Rust enums have
+   ```c
+   LDKChannelKeys InMemoryChannelKeys_as_ChannelKeys(const LDKInMemoryChannelKeys *this_arg);
+   ```
+
+ * Rust "unitary" enums are mapped simply as an equivalent C enum; however, some Rust enums have
    variants which contain payloads. Such enums are mapped automatically by cbindgen as a tag which
-   indicates the type and a union which holds the relevant fields for a given tag. A `X_free`
+   indicates the type and a union which holds the relevant fields for a given tag. An `X_free`
    function is provided for the enum as a whole which automatically frees the correct fields for a
    given tag, and a `Sentinel` tag is provided which causes the free function to do nothing (but
    which must never appear in an enum when accessed by Rust code). The `Sentinel` tag is used by
    the C++ wrapper classes to allow moving the ownership of an enum while invalidating the old copy.
 
    For example, the unitary enum `LDKChannelMonitorUpdateErr` is mapped as follows:
-   ```
+   ```c
    typedef enum LDKChannelMonitorUpdateErr {
       /** .. */
       LDKChannelMonitorUpdateErr_TemporaryFailure,
@@ -97,7 +103,7 @@ type getting an `LDK` prefix to their native Rust type names.
    ```
 
    and the non-unitary enum LDKErrorAction is mapped as follows:
-   ```
+   ```c
    typedef enum LDKErrorAction_Tag {
       /** .. */
       LDKErrorAction_DisconnectPeer,
@@ -126,29 +132,33 @@ type getting an `LDK` prefix to their native Rust type names.
    } LDKErrorAction;
    ```
 
- * Struct member functions are mapped as `Struct_function_name` and take a reference to the mapped
+ * Struct member functions are mapped as `Struct_function_name` and take a pointer to the mapped
    struct as their first argument. Free-standing functions are mapped simply as `function_name` and
    take the relevant mapped type arguments.
 
    Functions which return `&OpaqueRustType` and which return `OpaqueRustType` are both mapped to a
    function returning an owned wrapper struct. The `_underlying_ref` flag (see above) will be set
    to indicate that the pointed-to Rust object is owned or only a reference. Thus, when
    implementing a function which Rust will call or calling a Rust function, you should check the
-   Rust documentation for the function to determine which behavior is expected.
+   Rust documentation for the function to determine whether an owned or referenced object is
+   expected or returned.
 
    Similarly, when a function takes an `Option<RustType>` as a parameter or a return value, the C
    type is the same as if it took only `RustType`, with the `inner` field set to NULL to indicate
-   None. For example, `ChannelManager_create_channel` takes an `Option<LDKUserCOnfig>` not an
+   None. For example, `ChannelManager_create_channel` takes an `Option<LDKUserConfig>` not an
    `LDKUserConfig`, but its definition is:
-   ```
-   MUST_USE_RES LDKCResult_NoneAPIErrorZ ChannelManager_create_channel(const LDKChannelManager *this_arg, ..., LDKUserConfig override_config);
+   ```c
+   MUST_USE_RES ... ChannelManager_create_channel(const LDKChannelManager *this_arg, ..., LDKUserConfig override_config);
    ```
 
  * Various containers (Tuples, Vecs, Results, etc) are mapped into C structs of the form
    `LDKCContainerType_ContainerElementsZ`. Inner fields are often pointers, and in the case of
    primitive types, these may be allocated in C using the system allocator. See [the Rust docs on
    your platform's default System allocator](https://doc.rust-lang.org/std/alloc/struct.System.html)
-   for which allocator you must use.
+   for which allocator you must use. $ecursive containers are possible, and simply replace the
+   `ContainerElements` part with `InnerContainerType_InnerContainerElementsZ`, eg
+   `LDKCResult_C2Tuple_SignatureCVec_SignatureZZNoneZ` represents a
+   `Result<(Signature, Vec<Signature>), ()>`.
 
 As the bindings are auto-generated, the best resource for documentation on them is the native Rust
 docs available via `cargo doc` or [docs.rs/lightning](https://docs.rs/lightning).
@@ -171,7 +181,7 @@ C++-isms to make memory model correctness easier to achieve. They provide:
  * move constructors both from C++ classes and the original C struct, with the original object
    cleared to ensure destruction/`X_free` calls do not cause a double-free.
  * Move semantics via the () operator, returning the original C struct and clearing the C++ object.
-   This allows calls such as `C_function(cpp_object)` which works as expected with move semantics.
+   This allows calls such as `C_function(cpp_object)` which work as expected with move semantics.
 
 In general, you should prefer to use the C++ bindings if possible, as they make memory leaks and
 other violations somewhat easier to avoid. Note that, because the C functions are not redefined in
@@ -189,5 +199,17 @@ These include:
    with a call through a function pointer to get the local variable set correctly. Such functions
    have comments describing the potential race conditions.
 
+   For example, the `ChannelKeys::pubkeys() -> &ChannelPublicKeys` function is mapped as this:
+
+   ```c
+   typedef struct LDKChannelKeys {
+      ...
+      LDKChannelPublicKeys pubkeys;
+      /** ... */
+      void (*set_pubkeys)(const LDKChannelKeys*);
+	  ...
+   } LDKChannelKeys;
+   ```
+   
 **It is highly recommended that you test any code which relies on the C (or C++) bindings in
 valgrind, MemorySanitizer, or other similar tools to ensure correctness.**