Skip to content
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

nim-status-go shim #5

Closed
iurimatias opened this issue Jun 19, 2020 · 2 comments
Closed

nim-status-go shim #5

iurimatias opened this issue Jun 19, 2020 · 2 comments
Labels

Comments

@iurimatias
Copy link
Member

Nim-status-go shim

Introduction

As a first step a shim layer would allow status-react to start using the nim library as soon as possible, we can then add new apis or replace status-go apis with nim ones in a steady, iterative manner instead of a big branch approach. Since the API is the same, that library should work without almost any changes required from status-react.

Approach

Using status-go as library in nim has been shown in nim-status-client as seen here

proc hashMessage*(p0: cstring): cstring {.importc: "HashMessage".}

proc initKeystore*(keydir: cstring): cstring {.importc: "InitKeystore".}

proc openAccounts*(datadir: cstring): cstring {.importc: "OpenAccounts".}

proc multiAccountGenerateAndDeriveAddresses*(paramsJSON: cstring): cstring {.importc: "MultiAccountGenerateAndDeriveAddresses".}

proc multiAccountStoreDerivedAccounts*(paramsJSON: cstring): cstring {.importc: "MultiAccountStoreDerivedAccounts".}

proc multiAccountImportMnemonic*(paramsJSON: cstring): cstring {.importc: "MultiAccountImportMnemonic".}

proc MultiAccountImportPrivateKey*(paramsJSON: cstring): cstring {.importc: "MultiAccountImportPrivateKey".}

proc multiAccountDeriveAddresses*(paramsJSON: cstring): cstring {.importc: "MultiAccountDeriveAddresses".}

proc saveAccountAndLogin*(accountData: cstring, password: cstring, settingsJSON: cstring, configJSON: cstring, subaccountData: cstring): cstring {.importc: "SaveAccountAndLogin".}

proc callRPC*(inputJSON: cstring): cstring {.importc: "CallRPC".}

proc callPrivateRPC*(inputJSON: cstring): cstring {.importc: "CallPrivateRPC".}

proc addPeer*(peer: cstring): cstring {.importc: "AddPeer".}

proc setSignalEventCallback*(callback: SignalCallback) {.importc: "SetSignalEventCallback".}

proc sendTransaction*(jsonArgs: cstring, password: cstring): cstring {.importc: "SendTransaction".}

proc generateAlias*(p0: GoString): cstring {.importc: "GenerateAlias".}

proc identicon*(p0: GoString): cstring {.importc: "Identicon".}

proc login*(acctData: cstring, password: cstring): cstring {.importc: "Login".}

proc logout*(): cstring {.importc: "Logout".}

To get the shim working, we can create a striped down version of the build system of nim-status-client that only uses libstatus (i.e not QT nor any other libraries) and outputs an static library that then can be used in status-react.

to summarize:

    1. create simple nim project that interacts with status-go
    1. compile project as a static library
    1. integrate with status-react

Tests

There should be tests in nim for each of the exposed APIs to ensure they are working as expected.

Existing work

Potential problems / issues to explore

  • compiling it as a static library correctly
  • unclear if strings will work out of the box or not, there are two types of strings cstring and GoString, when re-exposing these apis we should make sure it still works as expected
  • receiving signals from status-go and passing them through the nim library to the client, unclear if a shim for setSignalEventCallback will work as expected

note: the spec part of this task will be added to status-im/specs later

@arnetheduck
Copy link
Member

Since the API is the same, that library should work without almost any changes required from status-react.

from a practical pov, it might make sense to to name the exported symbols slightly differently - in v0.1, we could prefix them with ni or whatever, and offer up versions that simply forward the call their corresponding go implementations - from there onwards, status-react will be completely ignorant of where things are implemented.

there is reason to put two shims in place - this is a very common strategy when doing language interop:

  • shim-n: hides status-go vs status-nim from nim consumers - offers the shim with nim types and "feeling"
  • shim-c: calls shim-n (is a "nim consumer") and translates that to a language-independent lowest-common-denominator approach in terms of what API is offered so that it's easy to import/export in any language - status-react or any other project.

as far as development goes, putting shim-c in place that calls a fully status-go-based shim-n is a risk-free way of injecting the shim functionality into the build of status-react.

@iurimatias
Copy link
Member Author

@Samyoul pointed out there is a document detailing the api currently used status-im/specs#136 this what we should use as a reference

michaelsbradleyjr added a commit that referenced this issue Aug 4, 2020
Introduce a Nim implementation of `hashMessage` situated in a reorganized
library with *Nim-oriented* and *C-oriented* entry points inspired by the
[shim strategy][shim-strat] suggested by @arnetheduck.

The goal of this approach (per @iurimatias, at least as I understood what was
discussed and tasked) is for Nim implementations of equivalent functionality to
eventually supersede existing [status-go][sgo] implementations.

These changes will benefit from feedback by @arnetheduck, @zah,
@stefantalpalaru, @vitaliy, other members of stimbus/desktop, et al. – my dev
experiences with Nim and C are quite limited to date. Some of the changes may
be problematic, unnecessary, suboptimal, etc. I may have reified the *"shim
strategy"* badly. Please shred this PR apart and lead me to the 💡
light. Thanks for your help!

N.B. `tests-nim/login.nim` and `tests-c/login.c` use loops that never
terminate (introduced prior to this PR). Future work will attempt to remedy
that shortcoming but it's out of scope for this PR.

The reorganized library can be grouped into two trees of `.nim` sources, but
note that `import` statements interlink them.

**Nim-oriented**
```
├── src
│   ├── lib
│   │   ├── shim.nim
│   │   └── util.nim
│   ├── lib.nim
│   ├── nim_status.nim
│   ├── shim.nim
│   └── types.nim
```

**C-oriented**
```
├── src
│   ├── c
│   │   ├── lib
│   │   │   └── shim.nim
│   │   ├── lib.nim
│   │   ├── nim_status.nim
│   │   ├── shim.nim
│   │   ├── sys.nim
│   │   └── types.nim
```

The key difference between the Nim sources in one tree and the other is that
`nim-status`'s Nim-oriented sources are intended to be consumed by other Nim
sources (e.g. [status-im/nim-status-client][nsc] via Nim's built-in `import`),
while its C-oriented sources are intended to be compiled to a C
library (e.g. `nim_status.a`) and then linked/called by other C/C++ code. To
that end, the former use `string` in their call signatures while the latter use
`cstring`. Along the same lines, the C-oriented sources may return pointers to
memory allocated with `c_malloc` such that it's up to the caller to free the
memory occupied by the return values.

Both `src/shim.nim` and `src/c/shim.nim` are pure shims around `status-go`, the
main difference being their call signatures.

Both `src/nim_status.nim` and `src/c/nim_status.nim` represent a hybrid (or
amalgam) of `src/shim.nim` + `src/lib/shim.nim` and `src/c/shim.nim` +
`src/c/lib/shim.nim`, respectively.

With `src/lib.nim` the intention is to implement requisite functionality in a
manner independent of `status-go`'s idioms. In `src/lib/shim.nim` the intention
is to wrap around `src/lib.nim` in a way that preserves `status-go`'s call
signatures.

It may be that `src/c/lib.nim` is mostly pointless relative to `src/lib.nim`,
in which case the former could be dropped. It's presently included as an
illustration of the intentions explained above.

`c_malloc` has been copied (in `src/c/sys.nim`) from the source of Nim's
`system/ansi_c` because `ansi_c` is an undocumented internal API.

A Nim template or pragma might be useful with respect to `c_malloc` used in
`src/c/*`, i.e. to cut down on repetition.

Use of `{.exportc.}` is limited (mostly) to the top-level
`src/c/nim_status.nim` to avoid imposing exported symbols on C-oriented library
consumers that may wish to compose an alternative to `src/c/nim_status.nim`.

With respect to the Nim implementation of `hashMessage`, both Nim-oriented and
C-oriented tests have been implemented. Whether that is really
necessary/desirable for all *"replacement `proc`s"* is probably worth
discussing.

Adjust `.gitignore` and refactor `Makefile` accordingly; apply some lessons
learned while working on [status-im/nim-status-client][nsc].

Closes #34.

[shim-strat]: #5 (comment)
[sgo]: https://github.com/status-im/status-go
[nsc]: https://github.com/status-im/nim-status-client
michaelsbradleyjr added a commit that referenced this issue Aug 5, 2020
Introduce a Nim implementation of `hashMessage` situated in a reorganized
library with *Nim-oriented* and *C-oriented* entry points inspired by the
[shim strategy][shim-strat] suggested by @arnetheduck.

The goal of this approach (per @iurimatias, at least as I understood what was
discussed and tasked) is for Nim implementations of equivalent functionality to
eventually supersede existing [status-go][sgo] implementations.

These changes will benefit from feedback by @arnetheduck, @zah,
@stefantalpalaru, @vitaliy, other members of stimbus/desktop, et al. – my dev
experiences with Nim and C are quite limited to date. Some of the changes may
be problematic, unnecessary, suboptimal, etc. I may have reified the *"shim
strategy"* badly. Please shred this PR apart and lead me to the 💡
light. Thanks for your help!

N.B. `tests-nim/login.nim` and `tests-c/login.c` use loops that never
terminate (introduced prior to this PR). Future work will attempt to remedy
that shortcoming but it's out of scope for this PR.

The reorganized library can be grouped into two trees of `.nim` sources, but
note that `import` statements interlink them.

**Nim-oriented**
```
├── src
│   ├── lib
│   │   ├── shim.nim
│   │   └── util.nim
│   ├── lib.nim
│   ├── nim_status.nim
│   ├── shim.nim
│   └── types.nim
```

**C-oriented**
```
├── src
│   ├── c
│   │   ├── lib
│   │   │   └── shim.nim
│   │   ├── lib.nim
│   │   ├── nim_status.nim
│   │   ├── shim.nim
│   │   ├── sys.nim
│   │   └── types.nim
```

The key difference between the Nim sources in one tree and the other is that
`nim-status`'s Nim-oriented sources are intended to be consumed by other Nim
sources (e.g. [status-im/nim-status-client][nsc] via Nim's built-in `import`),
while its C-oriented sources are intended to be compiled to a C
library (e.g. `nim_status.a`) and then linked/called by other C/C++ code. To
that end, the former use e.g. `string` in call signatures while the latter use
`cstring`. Along the same lines, the C-oriented `procs` may return pointers to
memory allocated with `c_malloc` such that it's up to the caller to free the
memory occupied by the return values.

Both `src/shim.nim` and `src/c/shim.nim` are pure shims around `status-go`, the
main difference being their call signatures.

Both `src/nim_status.nim` and `src/c/nim_status.nim` represent a hybrid (or
amalgam) of `src/shim.nim` + `src/lib/shim.nim` and `src/c/shim.nim` +
`src/c/lib/shim.nim`, respectively.

With `src/lib.nim` the intention is to implement requisite functionality in a
manner independent of `status-go`'s idioms. In `src/lib/shim.nim` the intention
is to wrap around `src/lib.nim` in a way that preserves `status-go`'s call
signatures.

It may be that `src/c/lib.nim` is mostly pointless relative to `src/lib.nim`,
in which case the former could be dropped. It's presently included as an
illustration of the intentions explained above.

`c_malloc` has been copied (in `src/c/sys.nim`) from the source of Nim's
`system/ansi_c` because `ansi_c` is an undocumented internal API.

A Nim template or pragma might be useful with respect to `c_malloc` used in
`src/c/*`, i.e. to cut down on repetition.

Use of `{.exportc.}` is limited (mostly) to the top-level
`src/c/nim_status.nim` to avoid imposing exported symbols on C-oriented library
consumers who may wish to compose an alternative to `src/c/nim_status.nim`.

With respect to the Nim implementation of `hashMessage`, both Nim-oriented and
C-oriented tests have been implemented. Whether that is really
necessary/desirable for all *replacement `proc`s* is probably worth discussing.

Adjust `.gitignore` and refactor `Makefile` accordingly; apply some lessons
learned while working on [status-im/nim-status-client][nsc].

Closes #34.

[shim-strat]: #5 (comment)
[sgo]: https://github.com/status-im/status-go
[nsc]: https://github.com/status-im/nim-status-client
michaelsbradleyjr added a commit that referenced this issue Aug 5, 2020
Introduce a Nim implementation of `hashMessage` situated in a reorganized
library with *Nim-oriented* and *C-oriented* entry points inspired by the
[shim strategy][shim-strat] suggested by @arnetheduck.

The goal of this approach (per @iurimatias, at least as I understood what was
discussed and tasked) is for Nim implementations of equivalent functionality to
eventually supersede existing [status-go][sgo] implementations.

These changes will benefit from feedback by @arnetheduck, @zah,
@stefantalpalaru, @siphiuel, other members of stimbus/desktop, et al. – my dev
experiences with Nim and C are quite limited to date. Some of the changes may
be problematic, unnecessary, suboptimal, etc. I may have reified the *"shim
strategy"* badly. Please shred this PR apart and lead me to the 💡
light. Thanks for your help!

N.B. `tests-nim/login.nim` and `tests-c/login.c` use loops that never
terminate (introduced prior to this PR). Future work will attempt to remedy
that shortcoming but it's out of scope for this PR.

The reorganized library can be grouped into two trees of `.nim` sources, but
note that `import` statements interlink them.

**Nim-oriented**
```
├── src
│   ├── lib
│   │   ├── shim.nim
│   │   └── util.nim
│   ├── lib.nim
│   ├── nim_status.nim
│   ├── shim.nim
│   └── types.nim
```

**C-oriented**
```
├── src
│   ├── c
│   │   ├── lib
│   │   │   └── shim.nim
│   │   ├── lib.nim
│   │   ├── nim_status.nim
│   │   ├── shim.nim
│   │   ├── sys.nim
│   │   └── types.nim
```

The key difference between the Nim sources in one tree and the other is that
`nim-status`'s Nim-oriented sources are intended to be consumed by other Nim
sources (e.g. [status-im/nim-status-client][nsc] via Nim's built-in `import`),
while its C-oriented sources are intended to be compiled to a C
library (e.g. `nim_status.a`) and then linked/called by other C/C++ code. To
that end, the former use e.g. `string` in call signatures while the latter use
`cstring`. Along the same lines, the C-oriented `procs` may return pointers to
memory allocated with `c_malloc` such that it's up to the caller to free the
memory occupied by the return values.

Both `src/shim.nim` and `src/c/shim.nim` are pure shims around `status-go`, the
main difference being their call signatures.

Both `src/nim_status.nim` and `src/c/nim_status.nim` represent a hybrid (or
amalgam) of `src/shim.nim` + `src/lib/shim.nim` and `src/c/shim.nim` +
`src/c/lib/shim.nim`, respectively.

With `src/lib.nim` the intention is to implement requisite functionality in a
manner independent of `status-go`'s idioms. In `src/lib/shim.nim` the intention
is to wrap around `src/lib.nim` in a way that preserves `status-go`'s call
signatures.

It may be that `src/c/lib.nim` is mostly pointless relative to `src/lib.nim`,
in which case the former could be dropped. It's presently included as an
illustration of the intentions explained above.

`c_malloc` has been copied (in `src/c/sys.nim`) from the source of Nim's
`system/ansi_c` because `ansi_c` is an undocumented internal API.

A Nim template or pragma might be useful with respect to `c_malloc` used in
`src/c/*`, i.e. to cut down on repetition.

Use of `{.exportc.}` is limited (mostly) to the top-level
`src/c/nim_status.nim` to avoid imposing exported symbols on C-oriented library
consumers who may wish to compose an alternative to `src/c/nim_status.nim`.

With respect to the Nim implementation of `hashMessage`, both Nim-oriented and
C-oriented tests have been implemented. Whether that is really
necessary/desirable for all *replacement `proc`s* is probably worth discussing.

Adjust `.gitignore` and refactor `Makefile` accordingly; apply some lessons
learned while working on [status-im/nim-status-client][nsc].

Closes #34.

[shim-strat]: #5 (comment)
[sgo]: https://github.com/status-im/status-go
[nsc]: https://github.com/status-im/nim-status-client
michaelsbradleyjr added a commit that referenced this issue Aug 10, 2020
Introduce a Nim implementation of `hashMessage` situated in a reorganized
library with *Nim-oriented* and *C-oriented* entry points inspired by the
[shim strategy][shim-strat] suggested by @arnetheduck.

The goal of this approach (per @iurimatias, at least as I understood what was
discussed and tasked) is for Nim implementations of equivalent functionality to
eventually supersede existing [status-go][sgo] implementations.

These changes will benefit from feedback by @arnetheduck, @zah,
@stefantalpalaru, @siphiuel, other members of stimbus/desktop, et al. – my dev
experiences with Nim and C are quite limited to date. Some of the changes may
be problematic, unnecessary, suboptimal, etc. I may have reified the *"shim
strategy"* badly. Please shred this PR apart and lead me to the 💡
light. Thanks for your help!

N.B. `tests-nim/login.nim` and `tests-c/login.c` use loops that never
terminate (introduced prior to this PR). Future work will attempt to remedy
that shortcoming but it's out of scope for this PR.

The reorganized library can be grouped into two trees of `.nim` sources, but
note that `import` statements interlink them.

**Nim-oriented**
```
├── src
│   ├── lib
│   │   ├── shim.nim
│   │   └── util.nim
│   ├── lib.nim
│   ├── nim_status.nim
│   ├── shim.nim
│   └── types.nim
```

**C-oriented**
```
├── src
│   ├── c
│   │   ├── lib
│   │   │   └── shim.nim
│   │   ├── lib.nim
│   │   ├── nim_status.nim
│   │   ├── shim.nim
│   │   ├── sys.nim
│   │   └── types.nim
```

The key difference between the Nim sources in one tree and the other is that
`nim-status`'s Nim-oriented sources are intended to be consumed by other Nim
sources (e.g. [status-im/nim-status-client][nsc] via Nim's built-in `import`),
while its C-oriented sources are intended to be compiled to a C
library (e.g. `nim_status.a`) and then linked/called by other C/C++ code. To
that end, the former use e.g. `string` in call signatures while the latter use
`cstring`. Along the same lines, the C-oriented `procs` may return pointers to
memory allocated with `c_malloc` such that it's up to the caller to free the
memory occupied by the return values.

Both `src/shim.nim` and `src/c/shim.nim` are pure shims around `status-go`, the
main difference being their call signatures.

Both `src/nim_status.nim` and `src/c/nim_status.nim` represent a hybrid (or
amalgam) of `src/shim.nim` + `src/lib/shim.nim` and `src/c/shim.nim` +
`src/c/lib/shim.nim`, respectively.

With `src/lib.nim` the intention is to implement requisite functionality in a
manner independent of `status-go`'s idioms. In `src/lib/shim.nim` the intention
is to wrap around `src/lib.nim` in a way that preserves `status-go`'s call
signatures.

It may be that `src/c/lib.nim` is mostly pointless relative to `src/lib.nim`,
in which case the former could be dropped. It's presently included as an
illustration of the intentions explained above.

`c_malloc` has been copied (in `src/c/sys.nim`) from the source of Nim's
`system/ansi_c` because `ansi_c` is an undocumented internal API.

A Nim template or pragma might be useful with respect to `c_malloc` used in
`src/c/*`, i.e. to cut down on repetition.

Use of `{.exportc.}` is limited (mostly) to the top-level
`src/c/nim_status.nim` to avoid imposing exported symbols on C-oriented library
consumers who may wish to compose an alternative to `src/c/nim_status.nim`.

With respect to the Nim implementation of `hashMessage`, both Nim-oriented and
C-oriented tests have been implemented. Whether that is really
necessary/desirable for all *replacement `proc`s* is probably worth discussing.

Adjust `.gitignore` and refactor `Makefile` accordingly; apply some lessons
learned while working on [status-im/nim-status-client][nsc].

Closes #34.

[shim-strat]: #5 (comment)
[sgo]: https://github.com/status-im/status-go
[nsc]: https://github.com/status-im/nim-status-client
michaelsbradleyjr added a commit that referenced this issue Aug 10, 2020
Introduce a Nim implementation of `hashMessage` situated in a reorganized
library with *Nim-oriented* and *C-oriented* entry points inspired by the
[shim strategy][shim-strat] suggested by @arnetheduck.

The goal of this approach (per @iurimatias, at least as I understood what was
discussed and tasked) is for Nim implementations of equivalent functionality to
eventually supersede existing [status-go][sgo] implementations.

These changes will benefit from feedback by @arnetheduck, @zah,
@stefantalpalaru, @siphiuel, other members of stimbus/desktop, et al. – my dev
experiences with Nim and C are quite limited to date. Some of the changes may
be problematic, unnecessary, suboptimal, etc. I may have reified the *"shim
strategy"* badly. Please shred this PR apart and lead me to the 💡
light. Thanks for your help!

N.B. `tests/nim/login.nim` and `tests/c/login.c` use loops that never
terminate (introduced prior to this PR). Future work will attempt to remedy
that shortcoming but it's out of scope for this PR.

The reorganized library can be grouped into two trees of `.nim` sources, but
note that `import` statements interlink some of them.

**Nim-oriented**
```
├── src
│   ├── nim_status
│   │   ├── go
│   │   │   └── shim.nim
│   │   ├── lib
│   │   │   ├── shim.nim
│   │   │   └── util.nim
│   │   ├── lib.nim
│   │   └── types.nim
│   └── nim_status.nim
```

**C-oriented**
```
├── src
│   ├── nim_status
│   │   ├── c
│   │   │   ├── go
│   │   │   │   └── shim.nim
│   │   │   ├── lib
│   │   │   │   └── shim.nim
│   │   │   ├── lib.nim
│   │   │   ├── nim_status.nim
│   │   │   └── sys.nim
```

The key difference between the Nim sources in one tree and the other is that
the Nim-oriented sources are intended to be consumed by other Nim
sources (e.g. [status-im/nim-status-client][nsc] via Nim's built-in `import`),
while the C-oriented sources are intended to be compiled to a C
library (e.g. `nim_status.a`) and then linked/called by other C code. To that
end, the former use e.g. `string` in call signatures while the latter use
`cstring`. Along the same lines, the C-oriented `procs` may return pointers to
memory allocated with `c_malloc` such that it's up to the caller to free the
memory occupied by the return values.

Both `src/nim_status/go/shim.nim` and `src/nim_status/c/go/shim.nim` are pure
shims around status-go, the main difference being their call signatures.

With `src/nim_status/lib.nim` the intention is to implement functionality in a
manner independent of status-go's idioms.

In `src/nim_status/[c/]lib/shim.nim` the intention is to wrap around
`src/nim_status/lib.nim` in a way that preserves status-go's call signatures
and formatting. For example, the `hashMessage` proc introduced in this PR in
`src/nim_status/lib.nim` returns a hex string like `0xabcd...` while
`lib/shim.nim` returns JSON that's a match for status-go:
`{"result":"0xabcd..."}`.

Both `src/nim_status.nim` and `src/nim_status/c/nim_status.nim` represent a
hybrid of `go/shim.nim` and `lib/shim.nim`. Again, the goal is that over time
more and more procs implemented in Nim replace the shims around status-go.

Callers that don't need to consume return values formatted according to
status-go conventions can use `lib.nim` directly, e.g. via `import
nim_status/lib` or by compiling `src/nim_status/c/lib.nim` and linking/calling
from C.

`c_malloc` has been copied (in `src/nim_status/c/sys.nim`) from the source of
Nim's `system/ansi_c` because `ansi_c` is an undocumented internal API.

A Nim template or pragma might be useful with respect to usgae of `c_malloc` in
`src/nim_status/c/*`, i.e. to cut down on repetition.

Use of `{.exportc.}` is limited to the `src/nim_status/c/nim_status.nim` hybrid
shim to avoid imposing exported symbols on C-oriented library consumers who may
wish to compose things in a different manner.

With respect to the Nim implementation of `hashMessage`, both Nim-oriented and
C-oriented tests have been implemented. Whether doubling up the tests is really
necessary/desirable for all `proc`s is probably worth discussing.

Adjust `.gitignore` and refactor `Makefile` accordingly; apply some lessons
learned while working on [status-im/nim-status-client][nsc].

Closes #34.

[shim-strat]: #5 (comment)
[sgo]: https://github.com/status-im/status-go
[nsc]: https://github.com/status-im/nim-status-client
michaelsbradleyjr added a commit that referenced this issue Aug 10, 2020
Introduce a Nim implementation of `hashMessage` situated in a reorganized
library with *Nim-oriented* and *C-oriented* entry points inspired by the
[shim strategy][shim-strat] suggested by @arnetheduck.

The goal of this approach (per @iurimatias, at least as I understood what was
discussed and tasked) is for Nim implementations of equivalent functionality to
eventually supersede existing [status-go][sgo] implementations.

These changes will benefit from feedback by @arnetheduck, @zah,
@stefantalpalaru, @siphiuel, other members of stimbus/desktop, et al. – my dev
experiences with Nim and C are quite limited to date. Some of the changes may
be problematic, unnecessary, suboptimal, etc. I may have reified the *"shim
strategy"* badly. Please shred this PR apart and lead me to the 💡
light. Thanks for your help!

N.B. `tests/nim/login.nim` and `tests/c/login.c` use loops that never
terminate (introduced prior to this PR). Future work will attempt to remedy
that shortcoming but it's out of scope for this PR.

The reorganized library can be grouped into two trees of `.nim` sources, but
note that `import` statements interlink some of them.

**Nim-oriented**
```
├── src
│   ├── nim_status
│   │   ├── go
│   │   │   └── shim.nim
│   │   ├── lib
│   │   │   ├── shim.nim
│   │   │   └── util.nim
│   │   ├── lib.nim
│   │   └── types.nim
│   └── nim_status.nim
```

**C-oriented**
```
├── src
│   ├── nim_status
│   │   ├── c
│   │   │   ├── go
│   │   │   │   └── shim.nim
│   │   │   ├── lib
│   │   │   │   └── shim.nim
│   │   │   ├── lib.nim
│   │   │   ├── nim_status.nim
│   │   │   └── sys.nim
```

The key difference between the Nim sources in one tree and the other is that
the Nim-oriented sources are intended to be consumed by other Nim
sources (e.g. [status-im/nim-status-client][nsc] via Nim's built-in `import`),
while the C-oriented sources are intended to be compiled to a C
library (e.g. `nim_status.a`) and then linked/called by other C code. To that
end, the former use e.g. `string` in call signatures while the latter use
`cstring`. Along the same lines, the C-oriented `proc`s may return pointers to
memory allocated with `c_malloc` such that it's up to the caller to free the
memory occupied by the return values.

Both `src/nim_status/go/shim.nim` and `src/nim_status/c/go/shim.nim` are pure
shims around status-go, the main difference being their call signatures.

With `src/nim_status/lib.nim` the intention is to implement functionality in a
manner independent of status-go's idioms.

In `src/nim_status/[c/]lib/shim.nim` the intention is to wrap around
`src/nim_status/lib.nim` in a way that preserves status-go's call signatures
and formatting. For example, the `hashMessage` proc introduced in this PR in
`src/nim_status/lib.nim` returns a hex string like `0xabcd...` while
`lib/shim.nim` returns JSON that's a match for status-go:
`{"result":"0xabcd..."}`.

Both `src/nim_status.nim` and `src/nim_status/c/nim_status.nim` represent a
hybrid of `go/shim.nim` and `lib/shim.nim`. Again, the goal is that over time
more and more `proc`s implemented in Nim replace the shims around status-go.

Callers that don't need to consume return values formatted according to
status-go conventions can use `lib.nim` directly, e.g. via `import
nim_status/lib` or by compiling `src/nim_status/c/lib.nim` and linking/calling
from C.

`c_malloc` has been copied (in `src/nim_status/c/sys.nim`) from the source of
Nim's `system/ansi_c` because `ansi_c` is an undocumented internal API.

A Nim template or pragma might be useful with respect to usgae of `c_malloc` in
`src/nim_status/c/*`, i.e. to cut down on repetition.

Use of `{.exportc.}` is limited to the `src/nim_status/c/nim_status.nim` hybrid
shim to avoid imposing exported symbols on C-oriented library consumers who may
wish to compose things in a different manner.

With respect to the Nim implementation of `hashMessage`, both Nim-oriented and
C-oriented tests have been implemented. Whether doubling up the tests is really
necessary/desirable for all `proc`s is probably worth discussing.

Adjust `.gitignore` and refactor `Makefile` accordingly; apply some lessons
learned while working on [status-im/nim-status-client][nsc].

Closes #34.

[shim-strat]: #5 (comment)
[sgo]: https://github.com/status-im/status-go
[nsc]: https://github.com/status-im/nim-status-client
michaelsbradleyjr added a commit that referenced this issue Aug 10, 2020
Introduce a Nim implementation of `hashMessage` situated in a reorganized
library with *Nim-oriented* and *C-oriented* entry points inspired by the
[shim strategy][shim-strat] suggested by @arnetheduck.

The goal of this approach (per @iurimatias, at least as I understood what was
discussed and tasked) is for Nim implementations of equivalent functionality to
eventually supersede existing [status-go][sgo] implementations.

These changes will benefit from feedback by @arnetheduck, @zah,
@stefantalpalaru, @siphiuel, other members of stimbus/desktop, et al. – my dev
experiences with Nim and C are quite limited to date. Some of the changes may
be problematic, unnecessary, suboptimal, etc. I may have reified the *"shim
strategy"* badly. Please shred this PR apart and lead me to the 💡
light. Thanks for your help!

N.B. `tests/nim/login.nim` and `tests/c/login.c` use loops that never
terminate (introduced prior to this PR). Future work will attempt to remedy
that shortcoming but it's out of scope for this PR.

The reorganized library can be grouped into two trees of `.nim` sources, but
note that `import` statements interlink some of them.

**Nim-oriented**
```
├── src
│   ├── nim_status
│   │   ├── go
│   │   │   └── shim.nim
│   │   ├── lib
│   │   │   ├── shim.nim
│   │   │   └── util.nim
│   │   ├── lib.nim
│   │   └── types.nim
│   └── nim_status.nim
```

**C-oriented**
```
├── src
│   ├── nim_status
│   │   ├── c
│   │   │   ├── go
│   │   │   │   └── shim.nim
│   │   │   ├── lib
│   │   │   │   └── shim.nim
│   │   │   ├── lib.nim
│   │   │   ├── nim_status.nim
│   │   │   └── sys.nim
```

The key difference between the Nim sources in one tree and the other is that
the Nim-oriented sources are intended to be consumed by other Nim
sources (e.g. [status-im/nim-status-client][nsc] via Nim's built-in `import`),
while the C-oriented sources are intended to be compiled to a C
library (e.g. `nim_status.a`) and then linked/called by other C code. To that
end, the former use e.g. `string` in call signatures while the latter use
`cstring`. Along the same lines, the C-oriented `proc`s may return pointers to
memory allocated with `c_malloc` such that it's up to the caller to free the
memory occupied by the return values.

Both `src/nim_status/go/shim.nim` and `src/nim_status/c/go/shim.nim` are pure
shims around status-go, the main difference being their call signatures.

With `src/nim_status/lib.nim` the intention is to implement functionality in a
manner independent of status-go's idioms.

In `src/nim_status/[c/]lib/shim.nim` the intention is to wrap around
`src/nim_status/lib.nim` in a way that preserves status-go's call signatures
and formatting. For example, the `hashMessage` proc introduced in this PR in
`src/nim_status/lib.nim` returns a hex string like `0xabcd...` while
`lib/shim.nim` returns JSON that's a match for status-go:
`{"result":"0xabcd..."}`.

Both `src/nim_status.nim` and `src/nim_status/c/nim_status.nim` represent a
hybrid of `go/shim.nim` and `lib/shim.nim`. Again, the goal is that over time
more and more `proc`s implemented in Nim replace the shims around status-go.

Callers that don't need to consume return values formatted according to
status-go conventions can use `lib.nim` directly, e.g. via `import
nim_status/lib` or by compiling `src/nim_status/c/lib.nim` and linking/calling
from C.

`c_malloc` has been copied (in `src/nim_status/c/sys.nim`) from the source of
Nim's `system/ansi_c` because `ansi_c` is an undocumented internal API.

A Nim template or pragma might be useful with respect to usgae of `c_malloc` in
`src/nim_status/c/*`, i.e. to cut down on repetition.

Use of `{.exportc.}` is limited to the `src/nim_status/c/nim_status.nim` hybrid
shim to avoid imposing exported symbols on C-oriented library consumers who may
wish to compose things in a different manner.

With respect to the Nim implementation of `hashMessage`, both Nim-oriented and
C-oriented tests have been implemented. Whether doubling up the tests is really
necessary/desirable for all `proc`s is probably worth discussing.

Adjust `.gitignore` and refactor `Makefile` accordingly; apply some lessons
learned while working on [status-im/nim-status-client][nsc].

Closes #34.

[shim-strat]: #5 (comment)
[sgo]: https://github.com/status-im/status-go
[nsc]: https://github.com/status-im/nim-status-client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants