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

App size #146

Merged
merged 13 commits into from
Feb 21, 2023
Merged

App size #146

merged 13 commits into from
Feb 21, 2023

Conversation

neithanmo
Copy link
Collaborator

@neithanmo neithanmo commented Feb 9, 2023

This PR aims to reduce the app size, applying some techniques that were used with another Rust app(AVAX):

🔗 zboto Link

@neithanmo neithanmo requested a review from ftheirs February 9, 2023 20:49
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job! To be fair the biggest improvement I saw on avalanche was removing rlib from the crate types, but it seems like it's not used here, so 🤷...

Anyways, good cleanup!

app/rust/src/parser/c32.rs Outdated Show resolved Hide resolved
app/rust/src/parser/c32.rs Outdated Show resolved Hide resolved
app/rust/src/parser/message.rs Outdated Show resolved Hide resolved
app/rust/src/parser/spending_condition.rs Outdated Show resolved Hide resolved
app/rust/src/parser/structured_msg.rs Outdated Show resolved Hide resolved
app/rust/src/parser/transaction_payload.rs Outdated Show resolved Hide resolved
app/rust/src/parser/transaction_payload.rs Outdated Show resolved Hide resolved
app/rust/src/parser/utils.rs Outdated Show resolved Hide resolved
@neithanmo neithanmo requested a review from a user February 15, 2023 15:38
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if those are better with iter_mut or were forgotten

app/rust/src/parser/c32.rs Outdated Show resolved Hide resolved
app/rust/src/parser/message.rs Outdated Show resolved Hide resolved
app/rust/src/parser/structured_msg.rs Outdated Show resolved Hide resolved
@neithanmo neithanmo merged commit 5f40d62 into main Feb 21, 2023
@ftheirs
Copy link
Contributor

ftheirs commented Feb 22, 2023

Closing this #145

neithanmo added a commit that referenced this pull request Feb 24, 2023
* avoid indexing to reduce binary size

use try call to avoid panic machinery to be added by the compiler

optimize for binary size and remove debug symbols

* update tests

* bump app version and update snapshots

* Derive Debug only under test

* Remove panic-halt to reduce app size

* Reduce calls to procedures that can cause panics

* Rename LedgerPanic to ApduPanic

* fix format

* use unreachable_unchecked

* some improvements in size

* fix return value

* Update zxlib

* use copy_from_slice and better error report
@carlosala carlosala deleted the app_size branch June 2, 2023 06:48
neithanmo added a commit that referenced this pull request Jul 24, 2024
* avoid indexing to reduce binary size

use try call to avoid panic machinery to be added by the compiler

optimize for binary size and remove debug symbols

* update tests

* bump app version and update snapshots

* Derive Debug only under test

* Remove panic-halt to reduce app size

* Reduce calls to procedures that can cause panics

* Rename LedgerPanic to ApduPanic

* fix format

* use unreachable_unchecked

* some improvements in size

* fix return value

* Update zxlib

* use copy_from_slice and better error report
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ledger Nano S screen showing incorrect address (40 instead of 41 characters)
2 participants