-
Notifications
You must be signed in to change notification settings - Fork 142
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
Fix contract deployments with Ledger #4683
Comments
@kyranjamie any idea what could be causing this? |
It could be that the stacks library that we are using for ledger doesn't support contract deploys. Opened an issue here: Zondax/ledger-stacks#153 |
Update: waiting for this PR to get merged before implementing it in the wallet: Zondax/ledger-stacks#155 |
Waiting for the new zondax/ledger-stacks app release to ledger store |
Removing from milestone because blocked |
@pete-watters does this appear to be the same issue as to #5115 to you? |
The error we show is similar but when I provided data from #5115 to Zondax they ran it through their simulator and it was working so they haven't added a fix for it in their latest version. On that issue they ask about the data we are sending that comes from the contract itself I have this issue open on their repo: Zondax/ledger-stacks#159 They helped me debug as I sent them the data we send to ledger and they ran tests on it and it worked OK in their simulator. From Leather we get an error code back from the Ledger and we don't show any prompts on the users Ledger: #5115 (comment) |
Got it, so should we close out this specific issue in favor of #5115 or are they still technically different? |
Lets keep this open and see if the Ledger App update resolves it. @edgarkhanzadian was waiting on that If that fixes this issue, we can close it then. If not lets analyse it with #5115 and see if they are the same. They could be having different failures for different reasons. In the UI we show the message |
According to @MicaiahReid contract deployments are rejected when the size of the transaction is equal or greater than 16390 bytes. When they use Xverse to deploy, smaller size transactions will succeed. Examples of raw payloads can be found in this source comment: Zondax/ledger-stacks#166 (comment) With Leather contract deployments fail with "your ledger device has rejected the payload stating it is invalid" with any size according to @lgalabru. Source: Zondax/ledger-stacks#166 (comment) |
I used a Ledger Nano S+ to deploy a small smart contract with Leather/Ledger and that works now. Example contract (default contract in stacks explorer sandbox):
Trying a contract with 500 lines fails which is expected due to the current memory limitations mentioned in the Ledger-stacks issue thread (166). |
Nice @314159265359879 ! Did something have to change to get this working, or was I doing something wrong when attempting to deploy a contract with Leather/Ledger? |
There is one change, this is the first time I tried this with Stacks app version 0.24.2 which was released yesterday. Prior to this I was using 0.23.3 and I also was under the impression that contract deployments with leather/ledger would fail regardless of the size. |
Yep, those were my findings as well. Glad it's fixed, thanks! |
@314159265359879 can we close this issue? |
Lets close this when the fix for big contracts is also live on Ledger Live/(Stacks App 0v24.4 or higher). The fix should improve things for Nano S+, X, Stax and Flex. Older models (Nano) does not have enough memory to change anything so if users encounter it there they may need to upgrade to one of these newer ledger devices to deploy a sizeable (or any) contract. This it the related PR (increase flash buffer size) Zondax/ledger-stacks#172 |
There has been an update on the Zondax ticket to indicate a fix for this has been merged by Ledger 🤞 |
Steps to reproduce:
Other steps that lead to the same outcome:
The text was updated successfully, but these errors were encountered: