nimbus-verified-proxy integration with status-go #1572

vitvly · 2023-05-03T15:28:27Z

This is depended on by https://github.com/siphiuel/lc-proxy-wrapper.

Custom nim-confutils dependency is due to adding a fix suggested in status-im/nim-confutils#31
.
Tracking issue: status-im/status-go#3282

Supersedes #1497

arnetheduck · 2023-05-08T15:37:19Z

nimbus_verified_proxy/nimbus_verified_proxy.nim

@@ -40,14 +40,27 @@ func getConfiguredChainId(networkMetadata: Eth2NetworkMetadata): Quantity =
  else:
    return networkMetadata.cfg.DEPOSIT_CHAIN_ID.Quantity

-proc run(config: VerifiedProxyConf) {.raises: [CatchableError].} =
+type OnHeaderCallback* = proc (s: cstring) {.cdecl.}


Suggested change

type OnHeaderCallback* = proc (s: cstring) {.cdecl.}

type OnHeaderCallback* = proc (s: cstring) {.cdecl, raises: [], gcsafe.}

arnetheduck · 2023-05-08T15:37:55Z

nimbus_verified_proxy/nimbus_verified_proxy.nim

+  echo "finalized header callback set"
+
+
+proc run(config: VerifiedProxyConf) {.raises: [CatchableError, Exception].} =


Suggested change

proc run(config: VerifiedProxyConf) {.raises: [CatchableError, Exception].} =

proc run(config: VerifiedProxyConf) {.raises: [CatchableError].} =

With this change, i get the following compiler error on that line:

Error: Exception can raise an unlisted exception: Exception

For this exception, you need to track down where the Exception is coming from - usually, it's the callback that I noted above should be annotated, else the compiler will assume the most broad exception raising.

You can figure it out by putting try/except around specific lines until it goes away: the proper fix is what I suggest above for the callbacks, to annotate them with raises: [], gcsafe

Thanks, figured it out - it was getCLIParams() that threw the exception.

arnetheduck · 2023-05-08T15:40:00Z

nimbus_verified_proxy/nimbus_verified_proxy.nim

+            notice "### Invoking finalizedHeaderCallback"
+            {.gcsafe.}:
+              try:
+                finalizedHeaderCallback(Json.encode(finalizedHeader))
+              except Exception as e:
+                notice "finalizedHeaderCallback exception"
+


Suggested change

notice "### Invoking finalizedHeaderCallback"

{.gcsafe.}:

try:

finalizedHeaderCallback(Json.encode(finalizedHeader))

except Exception as e:

notice "finalizedHeaderCallback exception"

notice "### Invoking finalizedHeaderCallback"

finalizedHeaderCallback(Json.encode(finalizedHeader))

@arnetheduck thank you for review!
The Json.encode() invocation throws SerializationError. If not wrapped with try-except the following compiler error is shown:

Error: writeValue writer`gensym1293, optimisticHeader can raise an unlisted exception: SerializationError

right, then the right approach is to specifically catch that (try: ... except SerializationError...).

Got it, thanks.

arnetheduck · 2023-05-08T15:40:13Z

nimbus_verified_proxy/nimbus_verified_proxy.nim

@@ -171,6 +192,14 @@ proc run(config: VerifiedProxyConf) {.raises: [CatchableError].} =
        info "New LC optimistic header",
          optimistic_header = shortLog(forkyHeader)
        optimisticProcessor.setOptimisticHeader(forkyHeader.beacon)
+      if optimisticHeaderCallback != nil:


vitvly · 2023-05-18T13:32:23Z

@arnetheduck can you please have a look at the fixes? Hopefully this looks better now.

vitvly · 2023-06-19T08:53:41Z

@arnetheduck @zah can you please review the fixes made by @Ivansete-status ?

arnetheduck · 2023-06-20T11:31:55Z

nimbus_verified_proxy/nimbus_verified_proxy.nim

+  echo "Quitting"
+
+proc NimMain() {.importc.}
+proc startProxyViaJson*(configJson: cstring) {.exportc, dynlib.} =


So, we need to think about this API in particular - when called from go, this would block the go thread until finished.

As an alternative to this model, I would propose the following API:

type Context = object thread: Thread[ptr Context] config: cstring stop: bool onFinalizedHeader: proc... proc runContext(ctx: ptr Context) {.thread.} = let node = newLightClientNode(ctx.config) while not ctx[].stop and node.running: let timeout = sleepAsync(100.millis) waitFor timeout # do cleanup node.stop() proc startLightClientProxy(config: cstring, onFinalizedHeader: ...): ptr Context = let ctx = createShared(Context, 1) ctx.config = cast[cstring](allocShared0(len(config) + 1)) ctx.onFinalizedHeader = ... copyMem(ctx.config, config, len(config)) createThread(ctx.thread, runContext, ctx) proc stopLightClientProxy(ctx: ptr Context) = ctx[].stop = false ctx.thread.joinThreads() deallocShared(ctx.config) ... freeShared(ctx)

In the above setup, the light client runs in its own thread and all "shared" memory is allocated inside the Context object which is freed when go wants to stop the proxy.

Instead of calling run(config) which calls runForever, we use a custom polling loop that wakes up every 100ms to check if we should shut down - later, we can replace this with a more sophisticated method but this is sufficient for now.

So, to rehash the key ideas:

go calls a "start" function that takes as parameter the configuration and any callbacks that are needed

the "start" function allocates a context which is given back to go - go doesn't interact with this context directly but it's used in the rest of the API

the "start" function does not block

the node operates as usual in its own thread - the only difference is the polling for the stop boolean that happens every now and then

notice in particular how the node instance is allocated inside the thread

when it's time to shut down, go calls the "stop" function giving the context - it blocks until the node has stopped

Later, the Context object will also hold a queue for interacting with the node from go - this queue will be used to make sure that there are no unsanctioned memory interactions between go and nim - it looks like so:

proc sendRequest(ctx: ptr Context, data: ptr byte, len: cint) = let copy = allocShared(len) copyMem(copy, data, len) ctx.queue.add((copy, len))

If a return value is needed, this is easiest to do via a callback:

proc sendRequest(ctx: ptr Context, data: ptr byte, len: cint, user: pointer, callback: proc(user: pointer, response: ptr byte, len: cint) {.cdecl.}) = let copy = allocShared(len) copyMem(copy, data, len) ctx.queue.add((copy, len, callback))

when the result of the call is ready, the callback is simply called making it easy to integrate with the async behavior of go

Confirming that cgo can be called from non-go threads: golang/go@6c97639

status-im/nim-style-guide#15 contains more examples - will be adding a go example soon as well

arnetheduck · 2023-12-04T15:28:37Z

let's move the code needed to compile the proxy as a library to a separate file like so:

Similar to the above two libraries, the C header file should be written manually to expose the functions - it's probably easier to create a static library instead of a dll/so and link everything statically.

We can also create the PR for confutils and get that merged ASAP so that it is not a blocker.

vitvly · 2024-01-15T08:52:44Z

Tried rebasing, but this causes issues related to Nim version i think. I start getting the below errors when building libverifproxy:

/Users/vvlasov/c/nimbus-eth1/nimbus/db/aristo/aristo_blobify.nim(193, 27) Error: undeclared field: 'keys' for type tables.Table [type declared in /Users/vvlasov/c/nimbus-eth1/vendor/nimbus-build-system/vendor/Nim/lib/pure/collections/tables.nim(206, 3)]

(pushed the rebased version to https://github.com/status-im/nimbus-eth1/tree/status_go_integration_rebase)

arnetheduck · 2024-01-15T08:54:42Z

Tried rebasing, but this causes issues

did you upgrade your copy of the dependencies (nim-stew etc) after rebasing?

vitvly · 2024-01-15T08:59:33Z

i think i did - i made sure that all submodule deps are identical to the ones in master, except for nim-confutils. Then did make mrproper, make update, make libverifproxy. Must be something that i've messed up, i guess.

vitvly · 2024-01-15T09:04:45Z

The draft PR for rebased version here: #1971

vitvly · 2024-01-25T10:36:57Z

Closing in favor of #1971

vitvly force-pushed the status_go_integration branch 2 times, most recently from 9fdf130 to 5d7a939 Compare May 8, 2023 13:58

vitvly mentioned this pull request May 8, 2023

Nimbus verified proxy integration status-im/status-go#3282

Open

6 tasks

arnetheduck reviewed May 8, 2023

View reviewed changes

vitvly force-pushed the status_go_integration branch 2 times, most recently from 349972d to 92da4e4 Compare May 15, 2023 16:14

arnetheduck reviewed Jun 20, 2023

View reviewed changes

vitvly force-pushed the status_go_integration branch from 130f246 to e04d676 Compare August 18, 2023 11:12

Nimbus light client integration with status-go

aa5199b

vitvly force-pushed the status_go_integration branch from 119fdaa to aa5199b Compare August 18, 2023 11:30

Vitaliy Vlasov and others added 2 commits August 28, 2023 17:25

Use header callback params

c848cf1

Use single headerCallback

aba4dbf

vitvly mentioned this pull request Nov 6, 2023

status-go integration: workflow questions #1882

Open

Move integration-specific code for verified_proxy

89bab20

arnetheduck mentioned this pull request Jan 17, 2024

Nimbus light client integration with status-go rebased version #1971

Merged

Add cleanup code, address review comments

b5d137e

vitvly closed this Jan 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

nimbus-verified-proxy integration with status-go #1572

nimbus-verified-proxy integration with status-go #1572

vitvly commented May 3, 2023

arnetheduck May 8, 2023 •

edited

Loading

vitvly May 9, 2023

arnetheduck May 8, 2023

vitvly May 9, 2023

arnetheduck May 9, 2023

vitvly May 15, 2023

arnetheduck May 8, 2023

vitvly May 9, 2023

arnetheduck May 9, 2023

vitvly May 9, 2023

vitvly May 15, 2023

arnetheduck May 8, 2023

vitvly May 15, 2023

vitvly commented May 18, 2023

vitvly commented Jun 19, 2023

arnetheduck Jun 20, 2023

arnetheduck Jun 20, 2023

arnetheduck Jun 21, 2023

arnetheduck commented Dec 4, 2023

vitvly commented Jan 15, 2024

arnetheduck commented Jan 15, 2024

vitvly commented Jan 15, 2024

vitvly commented Jan 15, 2024

vitvly commented Jan 25, 2024

	type OnHeaderCallback* = proc (s: cstring) {.cdecl.}
	type OnHeaderCallback* = proc (s: cstring) {.cdecl, raises: [], gcsafe.}

		echo "finalized header callback set"


		proc run(config: VerifiedProxyConf) {.raises: [CatchableError, Exception].} =

nimbus-verified-proxy integration with status-go #1572

nimbus-verified-proxy integration with status-go #1572

Conversation

vitvly commented May 3, 2023

arnetheduck May 8, 2023 • 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

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

Choose a reason for hiding this comment

vitvly commented May 18, 2023

vitvly commented Jun 19, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arnetheduck commented Dec 4, 2023

vitvly commented Jan 15, 2024

arnetheduck commented Jan 15, 2024

vitvly commented Jan 15, 2024

vitvly commented Jan 15, 2024

vitvly commented Jan 25, 2024

arnetheduck May 8, 2023 •

edited

Loading