search

SiaFoundation / app-sia-x

(WIP) Sia App for Ledger Nano X
MIT License
7 stars 4 forks source link

Make Back Button Work #4

Closed chris124567 closed 10 months ago

chris124567 commented 10 months ago

This PR allows us to use backward and forward navigation in the Stax app. To do that, it required changing the code that streams in the transaction to read the whole transaction in at once. Unfortunately this means we have to store a fixed size buffer of transaction elements. The code currently stores only the elements that we actually need to display.

The limit to how many elements you can store varies by device but when you make too the limit large, you get this kind of error when building:

ld.lld: error: section '.bss' will not fit in region 'SRAM': overflowed by 1228 bytes

For most devices (NanoX, NanoSP, Stax) I just settled on 64 maximum elements as 96 and 128 were too large. But the Nano S has a ridiculously small amount of RAM and can only store 6 elements. So I was unsure how to proceed here without causing large amounts of code duplication. I hope we are able to just leave the Nano S build based on the older version of the code as is.

chris124567 commented 10 months ago

Looks like some of the cryptography functions we used are now deprecated but not yet removed which is causing a bunch of CI errors. Will look into the replacements!

chris124567 commented 10 months ago

Alright, I think I got it. I am going to test this on a physical Nano S and figure out why the ragger tests aren't running tomorrow. Then this should be good to merge.

chris124567 commented 10 months ago

I was not able to test on my physical Ledger S because I couldn't get my computer to recognize the device so that I could flash the app despite trying a bunch of different things. But I don't see any reason why it wouldn't work given that speculos is a nearly perfect emulator.

chris124567 commented 10 months ago

Unless anyone has additional suggestions (which if there are any on the memory saving changes I just made I'd appreciate it) I think this should be OK to merge. Technically you can compile with a slightly higher element limit for the Nano S but the app will crash if you do more than one action. I did not encounter crashes at 22 elements (but I did at 24) so I went with 20 to have some margin of safety.

lukechampine commented 10 months ago

20 should be fine 👍🏻