Skip to content

Commit 89e9fb0

Browse files
authored
Merge pull request #4 from mintlayer/nanox_so_fix
Nanox so fix
2 parents 55badf3 + 223c0ea commit 89e9fb0

179 files changed

Lines changed: 346 additions & 129 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,3 +24,6 @@ __pycache__/
2424
ledger/
2525
# Build directory
2626
build/
27+
28+
# VSCode
29+
.vscode/

Cargo.lock

Lines changed: 6 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ edition = "2021"
55

66
[dependencies]
77
messages = { path = "./messages" }
8-
ledger_device_sdk = "1.34.0"
9-
ledger_secure_sdk_sys = "1.15.0"
8+
ledger_device_sdk = "1.35.1"
9+
ledger_secure_sdk_sys = "1.16.1"
1010
hex = { version = "0.4.3", default-features = false, features = ["alloc"] }
1111
bech32 = { version = "0.11", default-features = false, features = ["alloc"] }
1212
chrono = { version = "0.4", default-features = false, features = ["alloc"] }

README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,3 +143,7 @@ The following workflows are executed in [GitHub Actions](https://github.com/feat
143143
- Various lint checks :
144144
- Source code lint checks with `cargo fmt`
145145
- Python functional test code lint checks with `pylint` and `mypy`
146+
147+
## Additional documentation
148+
149+
For development guidelines related to the app's memory usage see [docs/memory_usage.md](docs/memory_usage.md).

docs/memory_usage.md

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# Memory usage
2+
3+
## Device memory size
4+
5+
Judging by `.ld` files in [the Ledger Rust SDK](https://github.com/LedgerHQ/ledger-device-rust-sdk/tree/cad196841dbd72c037cfa01bec81a4a3ae57a04e/ledger_secure_sdk_sys/devices),
6+
the amount of SRAM each model has is:
7+
| Device | SRAM |
8+
| ----------------- | ---- |
9+
| apex_p, nanosplus | 40KB |
10+
| flex, stax | 36KB |
11+
| nanox | 28KB |
12+
13+
The first part of the RAM will be occupied by the app's globals (one of which will be the heap used by the Rust code) and the rest is stack.
14+
15+
The `HEAP_SIZE` variable in `.cargo/config.toml` specifies the size of the Rust heap (which is just [a static array under the hood](https://github.com/LedgerHQ/ledger-device-rust-sdk/blob/cad196841dbd72c037cfa01bec81a4a3ae57a04e/ledger_secure_sdk_sys/src/lib.rs#L64)).
16+
17+
I.e. the bigger `HEAP_SIZE` is, the less is the stack. And with the `HEAP_SIZE` of 16KB, we'll have less than 12KB of stack at nanox.
18+
19+
## Reducing app stack usage
20+
21+
- Function's parameters and the return value consume stack space (unless the object is small enough to be put into a register).
22+
- Moving an object around inside the function body may increase stack consumption as well.
23+
24+
So,
25+
- Box large types if you need to pass/return them by value.
26+
- Avoid unboxing boxed large objects when passing them by value. E.g. even if a function only needs `LargeObj`,
27+
pass `Box<LargeObj>` to it anyway (which would be discouraged by the "normal" best practices), because passing it
28+
unboxed would increase the stack usage.\
29+
This includes the case when a member function consumes `self` - declare it as `self: Box<Self>` instead.
30+
- `sizeof` of 200 bytes is probably large enough. E.g. in the past boxing certain objects of roughly this size
31+
decreased stack usage by roughly 1.3KB (which is more than 10% of all stack space available on nanox).
32+
33+
### Determining the current stack usage of the app
34+
35+
Build the app with `emit-stack-sizes`:
36+
```
37+
RUSTFLAGS="-Z emit-stack-sizes" cargo ledger build nanox
38+
```
39+
After that you can use `llvm-readobj` to obtain sizes of stack frames of each function:
40+
```
41+
llvm-readobj --stack-sizes --demangle target/nanox/release/mintlayer-app
42+
```
43+
44+
You can also force `llvm-readobj` to emit json and use `jq` to sort the output by the stack size. E.g. the following
45+
will print 20 functions with the biggest stack frame size:
46+
```
47+
llvm-readobj --stack-sizes --demangle --elf-output-style=JSON target/nanox/release/mintlayer-app | jq -r '.[].StackSizes | sort_by(.Entry.Size) | reverse | .[:20][] | .Entry | "\(.Size)\t\(.Functions | join(", "))"'
48+
```
49+
50+
### Determining the actual available stack
51+
52+
At least in the current version of the SDK, the linker script emits symbols that
53+
can be used to determine the actual stack size, e.g. via `llvm-readelf`:
54+
```
55+
llvm-readelf -s target/nanox/release/mintlayer-app | rg '_stack|_estack'
56+
```
57+
Example output:
58+
```
59+
1581: da7a425c 0 NOTYPE GLOBAL DEFAULT 6 app_stack_canary
60+
1624: da7a7000 0 NOTYPE GLOBAL DEFAULT 6 _estack
61+
1697: da7a4260 0 NOTYPE GLOBAL DEFAULT 6 _stack
62+
```
63+
Here `_estack` is the end of the stack area, `_stack` is the beginning of it and `app_stack_canary` is a 4-byte marker
64+
placed just below `_stack` and used to detect stack overflows. The difference between `_estack` and `_stack` will be
65+
the stack size, in this case it's da7a7000-da7a4260=2DA0 (11680 in decimal).
66+
67+
### Other notes
68+
69+
This code:
70+
```
71+
fn foo(x: &X) {
72+
match x {
73+
X::A => { /*do stuff*/ },
74+
X::B => { /*do other stuff*/ },
75+
}
76+
}
77+
```
78+
may use more stack than:
79+
```
80+
fn foo(x: &X) {
81+
match x {
82+
X::A => stuff(),
83+
X::B => other_stuff(),
84+
}
85+
}
86+
87+
#[inline(never)] fn stuff() { /*do stuff*/ }
88+
#[inline(never)] fn other_stuff() { /*do other stuff*/ }
89+
```
90+
I.e. it seems that LLVM cannot always reuse stack slots between different branches of the `match`, and with bigger enums
91+
and bigger stack usage in each branch the overhead becomes bigger as well. So, splitting a large `match` into separate
92+
non-inlinable functions may be a way of reducing the app's stack usage, but this should probably be the last resort,
93+
because if all large objects are boxed, the stack usage in each branch should be relatively small, which will make
94+
the overhead relatively small as well.

messages/Cargo.toml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ edition = "2024"
1010
#
1111
# Fix reference: https://github.com/paritytech/parity-scale-codec/pull/751
1212
# This fix should be included in releases after version 3.7.5.
13+
# Note: normally we would enable the "chain-error" feature of parity-scale-codec to make decode errors
14+
# more informative. But in the Ledger app we never examine or even print those errors, so enabling this
15+
# feature would only increase the size of the binary and make the app use more stack during decoding.
1316
parity-scale-codec = { git = "https://github.com/paritytech/parity-scale-codec.git", rev = "5021525697edc0661591ebc71392c48d950a10b0", default-features = false, features = [
1417
"derive",
15-
"chain-error",
1618
] }
1719

1820
num_enum = { version = "0.7.5", default-features = false }

messages/src/lib.rs

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
// Required for using String, Vec, format!...
2121
extern crate alloc;
2222

23-
use alloc::vec::Vec;
23+
use alloc::{boxed::Box, vec::Vec};
2424
use core::iter::ExactSizeIterator;
2525

2626
use derive_more::Display;
@@ -95,9 +95,9 @@ pub struct SignMessageReq {
9595

9696
#[derive(Encode, Decode)]
9797
pub enum SignTxReq {
98-
Input(TxInputReq),
99-
InputCommitment(mlcp::SighashInputCommitment),
100-
Output(TxOutputReq),
98+
Input(Box<TxInputReq>),
99+
InputCommitment(Box<mlcp::SighashInputCommitment>),
100+
Output(Box<TxOutputReq>),
101101
NextSignature,
102102
}
103103

@@ -402,16 +402,24 @@ pub enum StatusWord {
402402
// Standard Ledger APDU Codes
403403
#[display("Success")]
404404
Ok = 0x9000,
405+
#[display("Nothing received")]
406+
NothingReceived = 0x6982,
405407
#[display("User cancelled")]
406408
Deny = 0x6985,
407409
#[display("CLA not supported")]
408410
ClaNotSupported = 0x6E00,
409-
#[display("Wrong P1/P2 parameters")]
410-
WrongP1P2 = 0x6B00,
411411
#[display("Instruction not supported")]
412-
InsNotSupported = 0x6D00,
412+
InsNotSupported = 0x6E01,
413+
#[display("Wrong P1/P2 parameters")]
414+
WrongP1P2 = 0x6E02,
413415
#[display("Wrong APDU length")]
414-
WrongApduLength = 0x6700,
416+
WrongApduLength = 0x6E03,
417+
#[display("Unknown")]
418+
Unknown = 0x6D00,
419+
#[display("Panic")]
420+
Panic = 0xE000,
421+
#[display("Device locked")]
422+
DeviceLocked = 0x5515,
415423

416424
// App Specific Errors (0xB...)
417425
#[display("Transaction display failed")]
@@ -454,6 +462,8 @@ pub enum StatusWord {
454462
MaxBufferLenExceeded = 0xB012,
455463
#[display("Different input commitment hash")]
456464
DifferentInputCommitmentHash = 0xB013,
465+
#[display("Invalid Timestamp")]
466+
InvalidTimestamp = 0xB014,
457467

458468
// Ecc Errors
459469
#[display("ECC Carry")]

src/app_ui/sign.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ fn transaction_title(tx_type: &Option<TxType>) -> &'static str {
159159
Some(TxType::Htlc) => "Sign create HTLC transaction",
160160
Some(TxType::CreateDelegation) => "Sign create delegation transaction",
161161
Some(TxType::DelegationStake) => "Sign stake delegation transaction",
162-
Some(TxType::DelegationWithdrawl) => "Sign withdrawal delegation transaction",
162+
Some(TxType::DelegationWithdrawal) => "Sign withdrawal delegation transaction",
163163
Some(TxType::CreateStakePool) => "Sign create stake pool transaction",
164164
Some(TxType::DecommissionStakePool) => "Sign decommission stake pool transaction",
165165
Some(TxType::CreateNft) => "Sign create NFT transaction",
@@ -478,7 +478,7 @@ fn format_input(input: &InputCommand, coin: CoinType) -> Result<FormatedOutput,
478478
if cfg!(any(target_os = "nanosplus", target_os = "nanox")) {
479479
("Del Wdrwl", address_short)
480480
} else {
481-
("Delegation withdrawl", address_short)
481+
("Delegation withdrawal", address_short)
482482
}
483483
}
484484
},

src/errors.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,18 @@ pub fn cx_err_to_status(e: CxError) -> StatusWord {
3939
CxError::GenericError => StatusWord::EccGenericError,
4040
}
4141
}
42+
43+
pub fn sdk_err_to_status(e: ledger_device_sdk::io::StatusWords) -> StatusWord {
44+
match e {
45+
ledger_device_sdk::io::StatusWords::Ok => StatusWord::Ok,
46+
ledger_device_sdk::io::StatusWords::BadCla => StatusWord::ClaNotSupported,
47+
ledger_device_sdk::io::StatusWords::NothingReceived => StatusWord::NothingReceived,
48+
ledger_device_sdk::io::StatusWords::BadIns => StatusWord::InsNotSupported,
49+
ledger_device_sdk::io::StatusWords::BadP1P2 => StatusWord::WrongP1P2,
50+
ledger_device_sdk::io::StatusWords::BadLen => StatusWord::WrongApduLength,
51+
ledger_device_sdk::io::StatusWords::UserCancelled => StatusWord::Deny,
52+
ledger_device_sdk::io::StatusWords::Unknown => StatusWord::Unknown,
53+
ledger_device_sdk::io::StatusWords::Panic => StatusWord::Panic,
54+
ledger_device_sdk::io::StatusWords::DeviceLocked => StatusWord::DeviceLocked,
55+
}
56+
}

0 commit comments

Comments
 (0)