Skip to content

Commit

Permalink
- Remove outdated installation and release guides. Keep only 1 README
Browse files Browse the repository at this point in the history
  • Loading branch information
ajinkyaraj-23 committed Feb 29, 2024
1 parent 2b776a6 commit 882d606
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 414 deletions.
101 changes: 74 additions & 27 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -1,44 +1,91 @@
# Contributing
The Exciting World of Ledger C
------------------------------

## Hacking with [Nix](https://nixos.org/nix/)
Knowing C will help you in this adventure. But not as much as it should. There are some fun twists when it comes to Ledger C. Explore them below. Memorize them. There *will* be a quiz...

The `nix/` folder contains helper scripts for working with the ledger via Nix.
### Exceptions

### Installing
`nix/install.sh` will install both the wallet and baking apps. Use
`nix/install.sh s baking` to install just the baking app or
`nix/install.sh s wallet` to install just the wallet.
C doesn't have them. So you don't have to think about bracketing, exception safety, RAII, try/catch, all that.

### Developing
Use `nix/env.sh <s or x>` to enter a shell where you can run `make` and it will just work. You can also pass a command instead, e.g. `nix/env.sh s --run "make clean SHELL=bash"`. All `make` commands should be prefixed with `SHELL=bash`. Check the Makefile to see what options exist
Well not on the Ledger. You have exceptions! Which means you also have out-of-band code paths, and you now have to worry about exception safety.

For development, use `nix/watch.sh s make APP=<tezos_baking|tezos_wallet>` to incrementally build on every change. Be sure to `nix/env.sh s --run "make clean SHELL=bash"` if you start watching a different `APP`.
You can `THROW` a `uint16_t` like this `THROW(0x9000)`.

#### Debugging
Set `DEBUG=1` in the `Makefile` to so that the user-defined `parse_error()` macro provides a line-number. For `printf` style debugging see [Ledger's official instructions](https://ledger.readthedocs.io/en/latest/userspace/debugging.html)
Handling exceptions looks like this.

### Building
To do a full Nix build run `nix/build.sh`. You can pass `nix-build` arguments to this to build specific attributes, e.g. `nix/build.sh -A nano.s.wallet`.
```c
volatile int something = 0;
BEGIN_TRY {
TRY {
//do something;
}
CATCH(EXC_PARSE_ERROR) {
//do something on parse error
}
CATCH_OTHER(e) {
THROW(e);
}
FINALLY { }
}
END_TRY;
```

### Using tezos-client
Set environment variable `TEZOS_LOG="client.signer.ledger -> debug"` when running tezos-client to get the byte-level IO being sent
directly to/from the ledger
Exceptions that make it all the way to the top of the application are caught and returned as status codes from the APDU.

#### Gotchas

### Editor Integration
1. If a variable will be accessed both outside and inside the `BEGIN_TRY`/`END_TRY` block it must be `volatile`. The compiler doesn't expect these shenanigans and will optimize incorrectly if you don't.
2. Do not `return` in the `TRY` block. It will cause the Ledger to crash. Instead use a `volatile` variable to capture the result you want to `return` at the end.
3. Don't try to leave out blocks like `CATCH_OTHER(e)` and `FINALLY`. I don't know if that will work right and it's not worth the risk.

#### Visual Studio Code
#### Implications

1. Install `llvm-vs-code-extensions.vscode-clangd` extension.
2. Run `nix/setup-vscode.sh` to create local configuration. Note that these files are non-relocatable so you need to rerun this if you move the directory.
3. Restart Visual Studio Code.
1. If you have some global state and an exception is thrown then, unless you do something about it, that global state will remain. That might be a *very bad thing*. As long as you use globals our way (see Globals Our Way) you should be safe.

### Releasing

`nix/build.sh -A nano.s.release.all`
### Globals Our Way

`nix/build.sh -A nano.x.release.all`
`static const` globals are fine. `static` non-const are not fine for two reasons:

### Notes on testing
1. If you try to initialize them (which you would want to do!) then the app will crash. For example `static int my_bool = 3;` crashes whenever you try to read or write `my_bool`...
2. Instead of getting initialized to 0 like the C standard says, they are initialized to `0xA5`. Yes this can cause the compiler to incorrectly optimize your code.

See `test/README.md`
So just don't use `static` non-const globals. Instead we have `globals.h` which defines a large `struct` wher you can put your globals. At the beginning of the application we `memset(&global, 0, sizeof(global))` to clear it all to zeros.

Anything inside of `global.apdu` will get cleared when an exception gets to the top of the app (see Exceptions). To benefit from this behavior you should never return an error code via the in-band way of sending bytes back. All errors should be sent via `THROW`.

### Relocation

When we said `static const` globals were fine, we meant that they were possible. There is
a major gotcha, however: if you initialize a `static const` value with a pointer to another
`static` or `static const` value, the pointers might be incorrect and require relocation.

For example:

```
static const char important_string[] = "Important!";
static const char **important_string_ptrs = { important_string, NULL };
const char *str1 = important_string_ptrs[0];
const char *str2 = important_string;
```

`str` will now have the wrong value. `str2` will not. The reason `str1`
has the wrong value is that the linker gets confused with a reference
from one `static const` variable to another `static` variable on this
platform. To resolve, you can use the `PIC` macro, which will fix broken
pointers but never break a good pointer. Because of this, you can use
it liberally and not have to worry about breaking anything:

```
static const char important_string[] = "Important!";
static const char **important_string_ptrs = { important_string, NULL };
const char *str1 = PIC(important_string_ptrs[0]); // necessary use of PIC
const char *str2 = PIC(important_string); // unnecessary but harmless use of PIC
```

Many of the UI functions call `PIC` for you, so just because a UI function
accepts a data structure, doesn't mean that data structure is valid.

### Dynamic Allocation

Nope. Don't even try. No `malloc`/`calloc`/`free`. Use globals (see Globals).
111 changes: 0 additions & 111 deletions MacInstallation.md

This file was deleted.

24 changes: 12 additions & 12 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ When you want to upgrade to a new version, whether you built it yourself from so
or whether it's a new release of the `app.hex` files, use the same commands as you did
to originally install it. As the keys are generated from the device's seeds and the
derivation paths, you will have the same keys with every version of this Ledger hardware wallet app,
so there is no need to re-import the keys with `octez-client`.
so there is no need to re-import the keys with `octez-client`. You may need to run command `octez-client setup ledger to bake for ...` again as HWM and chain information would be erased after reinstalling the app.

### Special Upgrading Considerations for Bakers

Expand All @@ -423,7 +423,7 @@ this command to remind the hardware wallet what key you intend to authorize for
also set the HWM:

```
$ octez-client setup ledger to bake for <ALIAS> --main-hwm <HWM>
$ octez-client setup ledger to bake for ledger_<...> --main-hwm <HWM>
```

Alternatively, you can also set the High Watermark to the level of the most recently baked block with a separate command:
Expand Down Expand Up @@ -451,7 +451,7 @@ You currently cannot directly import a fundraiser account to the Ledger device.
### Two Ledger Devices at the Same Time

Two Ledger devices with the same seed should not ever be plugged in at the same time. This confuses
`tezos-client` and other client programs. Instead, you should plug only one of a set of paired
`octez-client` and other client programs. Instead, you should plug only one of a set of paired
ledgers at a time. Two Ledger devices of different seeds are fine and are fully supported,
and the computer will automatically determine which one to send information to.

Expand All @@ -463,7 +463,7 @@ computer for wallet transactions.
### unexpected seq num

```
$ client/bin/tezos-client list connected ledgers
$ octez-client list connected ledgers
Fatal error: Header.check: unexpected seq num
```

Expand All @@ -472,7 +472,7 @@ This means you do not have the Tezos application open on your device.
### No device found

```
$ tezos-client list connected ledgers
$ octez-client list connected ledgers
No device found.
Make sure a Ledger device is connected and in the Tezos Wallet app.
```
Expand All @@ -482,30 +482,30 @@ mean that your udev rules are not set up correctly.

### Unrecognized command

If you see an `Unrecognized command` error, it might be because there is no node for `tezos-client`
If you see an `Unrecognized command` error, it might be because there is no node for `octez-client`
to connect to. Please ensure that you are running a node. `ps aux | grep tezos-node` should display
the process information for the current node. If it displays nothing, or just displays a `grep`
command, then there is no node running on your machine.

### Error "Unexpected sequence number (expected 0, got 191)" on macOS

If `tezos-client` on macOS intermittently fails with an error that looks like
If `octez-client` on macOS intermittently fails with an error that looks like

```
client.signer.ledger: APDU level error: Unexpected sequence number (expected 0, got 191)
```

then your installation of `tezos-client` was built with an older version of HIDAPI that doesn't work well with macOS (see [#30](https://github.com/obsidiansystems/ledger-app-tezos/issues/30)).
then your installation of `octez-client` was built with an older version of HIDAPI that doesn't work well with macOS (see [#30](https://github.com/obsidiansystems/ledger-app-tezos/issues/30)).

To fix this you need to get the yet-unreleased fixes from the [HIDAPI library](https://github.com/signal11/hidapi) and rebuild `tezos-client`.
To fix this you need to get the yet-unreleased fixes from the [HIDAPI library](https://github.com/signal11/hidapi) and rebuild `octez-client`.

If you got HIDAPI from Homebrew, you can update to the `master` branch of HIDAPI like this:

```shell
$ brew install hidapi --HEAD
```

Then start a full rebuild of `tezos-client` with HIDAPI's `master` branch:
Then start a full rebuild of `octez-client` with HIDAPI's `master` branch:

```shell
$ brew unlink hidapi # remove the current one
Expand All @@ -518,10 +518,10 @@ Finally, rebuild `ocaml-hidapi` with Tezos. In the `tezos` repository:
```shell
$ opam reinstall hidapi
$ make all build-test
$ ./tezos-client list connected ledgers # should now work consistently
$ ./octez-client list connected ledgers # should now work consistently
```

Note that you may still see warnings similar to `Unexpected sequence number (expected 0, got 191)` even after this update. The reason is that there is a separate, more cosmetic, issue in `tezos-client` itself which has already been fixed but may not be in your branch yet (see the [merge request](https://gitlab.com/tezos/tezos/merge_requests/600)).
Note that you may still see warnings similar to `Unexpected sequence number (expected 0, got 191)` even after this update. The reason is that there is a separate, more cosmetic, issue in `octez-client` itself which has already been fixed but may not be in your branch yet (see the [merge request](https://gitlab.com/tezos/tezos/merge_requests/600)).

### Command Line Installations: "This app is not genuine"

Expand Down
Loading

0 comments on commit 882d606

Please sign in to comment.