feat: build and run F3 sidecar in Forest process via FFI #4727

hanabi1224 · 2024-09-04T06:16:58Z

Summary of changes

Changes introduced in this pull request:

integrate Go F3 sidecar into Forest via FFI(rust2go)
skip Go FFI on docs.rs build
docs on how to run F3 sidecar with FFI
TODO: build F3 sidecar via FFI in Dockerfile: Update Forest Dockerfile to build F3 sidecar via FFI #4758
TODO: F3 in CI devnet checks is updated in chore(ci): run F3 sidecar with FFI in devnet checks #4732

Reference issue to close (if applicable)

Closes #4700

Change checklist

I have performed a self-review of my own code,
I have made corresponding changes to the documentation,
I have added tests that prove my fix is effective or that my feature works (if possible),
I have made sure the CHANGELOG is up-to-date. All user-facing changes should be reflected in this document.

hanabi1224 · 2024-09-04T06:42:39Z

f3-sidecar/ffi_gen.go

@@ -0,0 +1,210 @@
+package main


Include this file so that the Go mod builds out of box without codegen from Rust build

LesnyRumcajs · 2024-09-05T12:40:27Z

src/f3/mod.rs

+    // Opt-out building the F3 sidecar staticlib
+    match std::env::var("FOREST_F3_SIDECAR_FFI_ENABLED") {
+        Ok(value) => {
+            let enabled = matches!(value.to_lowercase().as_str(), "1" | "true");


I recommend using

forest/src/utils/misc/env.rs

Lines 15 to 21 in b7c004b

/// Check if the given environment variable is set to truthy value.

pub fn is_env_truthy(env: &str) -> bool {

match std::env::var(env) {

Ok(var) => matches!(var.to_lowercase().as_str(), "1" | "true"),

_ => false,

}

}

when checking for truthy env variables.

Didn't know that. Fixed.

LesnyRumcajs · 2024-09-05T12:43:39Z

src/daemon/mod.rs

+                crate::rpc::f3::get_f3_rpc_endpoint().to_string(),
+                finality,
+                std::env::var("FOREST_F3_DB_PATH")
+                    .unwrap_or_else(|_| format!("/var/tmp/f3-db-{chain}")),


Why that location and not in the Forest data directory?

LesnyRumcajs · 2024-09-05T12:44:35Z

src/daemon/mod.rs

+        services.spawn_blocking(move || {
+            crate::f3::run_f3_sidecar_if_enabled(
+                format!("http://{rpc_address}/rpc/v1"),
+                crate::rpc::f3::get_f3_rpc_endpoint().to_string(),


In general, it would be nice if get_f3_rpc_endpoint would return a Url and the strong typing would be used in other places.

Changed to returning SocketAddr

@LesnyRumcajs Hmm this is not a rust SocketAddr because it supports DNS like f3:23456, nor is it an URL because protocol(http) is not included. Any suggestions on what type to use instead of String?

// Go code listener, err := net.Listen("tcp", f3RpcSocketAddress)

That's a weird one. We could make an enum, but if it's too much hassle and overengineering we could do with a type alias F3Endpoint for the time being. It's not blocking.

Thanks. I'd do more investigation and improve this in another PR. #4736

LesnyRumcajs · 2024-09-05T12:48:04Z

.github/workflows/forest.yml

+        env:
+          FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT: 1


Should it be noted somewhere that the crates.io releases do not contain this, as opposed to docker images?

It's only excluded from docs.rs build not crates.io. cargo install filecoin-forest still has sidecar FFI included

Or maybe I should just install Go for this job

LesnyRumcajs · 2024-09-06T08:14:02Z

.github/workflows/forest.yml

Should we mention that Go is required in https://github.com/ChainSafe/forest?tab=readme-ov-file#dependencies?

Good catch. Updated.

LesnyRumcajs · 2024-09-06T08:15:23Z

build.rs

+    match std::env::var("FOREST_F3_SIDECAR_FFI_BUILD_OPT_OUT") {
+        Ok(value) => !matches!(value.to_lowercase().as_str(), "1" | "true"),
+        _ => true,
+    }


Can we use the function mentioned elsewhere in the PR for that in the build script?

The util is not available here in the build script context

ruseinov · 2024-09-10T09:52:09Z

Looks fine by me, though the conditions are a bit confusing.

Curious if we shouldn't actually disable sidecar by default and make it feature-enabled. Perhaps then it's going to be less confusing with all the environment variables.

hanabi1224 · 2024-09-10T10:06:56Z

Curious if we shouldn't actually disable sidecar by default and make it feature-enabled. Perhaps then it's going to be less confusing with all the environment variables.

@ruseinov F3 is not yet feature complete for being enabled by default (in Lotus as well). e.g. Loading power table at a fixed upgrade epoch that is 2000 epochs older than head does not work. For now we can only do joint testing with some manual setup.

elmattic · 2024-09-11T11:00:17Z

src/f3/mod.rs

+        }
+        else {
+            if enabled {
+                tracing::error!("Failed to enable F3 sidecar, the forerst binary is not compiled with f3-sidecar Go lib");


typo in Forest name

Good catch! Will fix this in the follow up PR #4732

hanabi1224 force-pushed the hm/f3-sidecar-ffi branch 3 times, most recently from 9f41432 to 0963e25 Compare September 4, 2024 06:41

hanabi1224 commented Sep 4, 2024

View reviewed changes

hanabi1224 force-pushed the hm/f3-sidecar-ffi branch 2 times, most recently from e91f8e9 to 2854a58 Compare September 4, 2024 07:06

feat: build and run F3 sidecar in Forest process via FFI

7b4642b

hanabi1224 force-pushed the hm/f3-sidecar-ffi branch from 2854a58 to 7b4642b Compare September 4, 2024 07:20

Merge branch 'main' into hm/f3-sidecar-ffi

356ceb3

hanabi1224 mentioned this pull request Sep 5, 2024

chore(ci): run F3 sidecar with FFI in devnet checks #4732

Merged

4 tasks

hanabi1224 marked this pull request as ready for review September 5, 2024 10:05

hanabi1224 requested a review from a team as a code owner September 5, 2024 10:05

hanabi1224 requested review from LesnyRumcajs and elmattic and removed request for a team September 5, 2024 10:05

hanabi1224 force-pushed the hm/f3-sidecar-ffi branch 2 times, most recently from a32b3c2 to 8569f43 Compare September 5, 2024 10:36

install Go in Dockerfile

2640a6f

hanabi1224 force-pushed the hm/f3-sidecar-ffi branch from 8569f43 to 2640a6f Compare September 5, 2024 10:51

Merge branch 'main' into hm/f3-sidecar-ffi

b7c004b

LesnyRumcajs reviewed Sep 5, 2024

View reviewed changes

hanabi1224 force-pushed the hm/f3-sidecar-ffi branch 4 times, most recently from e7178ff to 1fbc03f Compare September 6, 2024 04:40

resolve comments

8d798ca

hanabi1224 force-pushed the hm/f3-sidecar-ffi branch from 1fbc03f to 8d798ca Compare September 6, 2024 05:03

hanabi1224 added 3 commits September 6, 2024 16:00

Merge branch 'main' into hm/f3-sidecar-ffi

8ab1f9c

revert SocketAddr change

e5e73bc

Merge remote-tracking branch 'origin/main' into hm/f3-sidecar-ffi

a6498c5

LesnyRumcajs reviewed Sep 6, 2024

View reviewed changes

hanabi1224 mentioned this pull request Sep 6, 2024

it would be nice if get_f3_rpc_endpoint would return a Url #4736

Open

LesnyRumcajs approved these changes Sep 6, 2024

View reviewed changes

hanabi1224 added 6 commits September 6, 2024 16:29

md updates

9fc2ba8

fix f3-db path

1018b7f

Merge branch 'main' into hm/f3-sidecar-ffi

2113f95

set GOWORK=off in build.rs

936c448

Merge remote-tracking branch 'origin/main' into hm/f3-sidecar-ffi

3080061

Merge branch 'main' into hm/f3-sidecar-ffi

293d5ff

hanabi1224 added 2 commits September 10, 2024 19:24

Merge branch 'main' into hm/f3-sidecar-ffi

a99c8c1

Merge remote-tracking branch 'origin/main' into hm/f3-sidecar-ffi

eff7286

hanabi1224 mentioned this pull request Sep 11, 2024

Update Forest Dockerfile to build F3 sidecar via FFI #4758

Open

Opt out F3 sidecar build in Dockerfile

bb05630

elmattic approved these changes Sep 11, 2024

View reviewed changes

hanabi1224 added this pull request to the merge queue Sep 11, 2024

Merged via the queue into main with commit 5257127 Sep 11, 2024
30 checks passed

hanabi1224 deleted the hm/f3-sidecar-ffi branch September 11, 2024 11:39

hanabi1224 mentioned this pull request Sep 12, 2024

Integrate F3 sidecar into Forest with FFI #4700

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: build and run F3 sidecar in Forest process via FFI #4727

feat: build and run F3 sidecar in Forest process via FFI #4727

hanabi1224 commented Sep 4, 2024 •

edited

Loading

hanabi1224 Sep 4, 2024

LesnyRumcajs Sep 5, 2024

hanabi1224 Sep 6, 2024 •

edited

Loading

LesnyRumcajs Sep 5, 2024

hanabi1224 Sep 6, 2024

LesnyRumcajs Sep 5, 2024

hanabi1224 Sep 6, 2024

hanabi1224 Sep 6, 2024 •

edited

Loading

LesnyRumcajs Sep 6, 2024

hanabi1224 Sep 6, 2024

LesnyRumcajs Sep 5, 2024

hanabi1224 Sep 5, 2024 •

edited

Loading

hanabi1224 Sep 6, 2024

LesnyRumcajs Sep 6, 2024

hanabi1224 Sep 6, 2024

LesnyRumcajs Sep 6, 2024

hanabi1224 Sep 6, 2024

ruseinov commented Sep 10, 2024

hanabi1224 commented Sep 10, 2024 •

edited

Loading

elmattic Sep 11, 2024

hanabi1224 Sep 11, 2024

	/// Check if the given environment variable is set to truthy value.
	pub fn is_env_truthy(env: &str) -> bool {
	match std::env::var(env) {
	Ok(var) => matches!(var.to_lowercase().as_str(), "1" \| "true"),
	_ => false,
	}
	}

feat: build and run F3 sidecar in Forest process via FFI #4727

feat: build and run F3 sidecar in Forest process via FFI #4727

Conversation

hanabi1224 commented Sep 4, 2024 • edited Loading

Summary of changes

Reference issue to close (if applicable)

Other information and links

Change checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanabi1224 Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanabi1224 Sep 6, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanabi1224 Sep 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ruseinov commented Sep 10, 2024

hanabi1224 commented Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanabi1224 commented Sep 4, 2024 •

edited

Loading

hanabi1224 Sep 6, 2024 •

edited

Loading

hanabi1224 Sep 6, 2024 •

edited

Loading

hanabi1224 Sep 5, 2024 •

edited

Loading

hanabi1224 commented Sep 10, 2024 •

edited

Loading