-
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 Ledger error for Stacks contract executions: opening an arkadiko vault ("Your ledger device has rejected the payload stating it is invalid") #5115
Comments
This was tested with Ledger nano S plus (firmware v1.1.1), And Ledger Nano X, with latest firmware. Leather version 6.30.1 and 6.31.0 |
This might be related to this issue? |
Thanks for sharing! 5113 looks related to bitcoin/PSBT operations, while this is related to Stacks network operations. They are likely unrelated. |
That said we do want to do a full audit of the wallet with Ledger this week. |
Possibly resolved with #2725? Retested today Deposit and withdraw, I tested and confirmed to fail with "rejected payload" error too. |
Users are reporting this same issue when visiting Gamma Stacks NFT marketplace and trying to use Ledger. I've tested on Gamma and confirmed the error. 'There's a technical issue when we try to access our account or manage collection using Leather (connected to Ledger Nano X) on PC. When we click on 'sign message,' on Gamma profile page, it returns this error: 'Your Ledger device has rejected the payload, stating it is invalid.' It occurs only on Stacks. The Bitcoin Gamma it's ok. Tested with: Chrome: v.124.0.6367.63, Leather: v.6.36, Ledger Live: v.2.79.1, Windows 10 Pro: v.22H2' |
Also on stackingdao btw |
Thanks for the reports and apologies this isn't working. I'm doing a full audit of ledger with Leather right now and and will investigate this soon |
@pete-watters thanks! the same error also happens when trying to generate a signature on https://app.stackingdao.com/signer?pool=SP4SZE494VC2YC5JYG7AYFQ44F5Q4PYV7DVMDPBG.stacking-pool-v1 - would be great if you can try it (just click on either of the "Save Commit Signature" buttons and sign the message) |
Thanks @philiphacks . I took a quick look to triage and I can indeed re-reproduce the error. We are showing this error as were are getting a
I also see in the console of https://app.stackingdao.com/ that we are displaying a deprecation warning and recommending to instead use
I was reproducing and trying to gather more information to see whats going wrong but now when I am clicking Have you seen that? I am not sure if the above is helpful and I will keep investigating but just sharing these initial findings |
@philiphacks - did you have a chance to check #5115 (comment)? According to the issue description it seems like this was working prior to arkadiko-dao/arkadiko#557 so perhaps it's not an issue with Leather? |
This has also been reported by ALEX with Leather version 6.41.1 👇
|
I'll have a look at this again but afaik this is not Arkadiko-related, I will try to upgrade our dependencies though |
@pete-watters has filed here for resolution with the Stacks app, since it appears that the functionality is broken in Leather as a downstream effect of a Ledger-side bug. |
Zondax replied on their issue and said that there are many changes in Zondax/ledger-stacks#158 and Zondax/ledger-stacks#156 might solve those issues. It is possible to test them in a real device using the app/pkg/[email protected] installers They asked:
I will test this and gather the information needed soon but if anyone here already has it to hand please comment and I can relay it to them. |
I am working with Zondax to try and get to the bottom of this. I have provided them with details of how to replicate the problem on Stacking Dao. I confirmed that the problem does also occur on I provided them with the payload we are sending when following these steps:
They replied to say:
I will try and get the serialised data sent to help them diagnose further |
I passed on some more information to Zondax: Zondax/ledger-stacks#159 (comment) They said:
|
Hi @ALL
However, that data passed our tests and we do not see any issue, As we are preparing for a release, I would rather want to revisit this in order to apply any fixes if necessary, can anyone help by providing similar information as above that we can use to reproduce the issue? |
@neithanmo Is there an idiot's guide to finding the domain and payload? My derivation path in Ledger is the same as yours, but I'm still getting the error regardless, when I try to open an Arkadiko vault. I'd be happy to provide my data, but I'm not sure how to find those two Leather version 6.44.0 |
Hi @Dior-Aranel , I obtained the payload and domain by:
You could add a breakpoint to get that information or else I can get it. You are trying to open a Vault right? Similar to #5115 (comment)? I think it could be worth it to update to |
I have gathered info reproducing this bug when trying to create a Vault in Arkadiko. When creating a vault it's slightly different and we use
For
When calling
|
Correct, I am trying to open a vault similar to the comment you linked I tried going through the steps you listed here The error message I got was: "Your Ledger device has rejected the payload stating it is invalid". When I try to open an Arkadiko vault, that's the same error message I get I'm still stuck on trying find the domain and payload. I'm a newbie when it comes to digging through Chrome DevTools |
This is a structured message, so You should use
that is weird, i used your data in a test that calls const signatureRequest = app.sign_structured_msg(
path,
'0c0000000308636861696e2d69640100000000000000000000000000000001046e616d650d0000000c706f782d342d7369676e65720776657273696f6e0d00000005312e302e30',
'0c0000000607617574682d696401000000000000000000000000000ca5640a6d61782d616d6f756e7401000000000000003635c9acdd09faf00006706572696f64010000000000000000000000000000000108706f782d616464720c000000020968617368627974657302000000142fffa9a09bb7fa7dced44834d77ee81c49c5f0cc0776657273696f6e0200000001040c7265776172642d6379636c65010000000000000000000000000000005905746f7069630d0000000a6167672d636f6d6d6974',
)
// Wait until we are not in the main menu
await sim.waitUntilScreenIsNot(sim.getMainMenuSnapshot())
await sim.compareSnapshotsAndApprove('.', `${m.prefix.toLowerCase()}-sign-structured_data_${name}`)
// Check the signature
const signature = await signatureRequest
console.log(signature)
expect(signature.returnCode).toEqual(0x9000)
expect(signature.errorMessage).toEqual('No errors') no errors |
I would give this payload:
a try while still using the domain above. |
Ok this payload triggers an error, i am assuming it does not contains a domain.
So, after parsing the payload as a clarity value, the parser found unexpected data in the input buffer. |
Is the payload not a tuple?
it starts with 1, which is the clarity value identifier for uint:
that is why the parse reads that and finishes, leaving remaining data in the buffer which triggers an error. in the sip018 documentation, the following is recommended: even though our implementation reads any clarity value type, it is better to have a tuple, in the provided payload, we read an integer and return, we do not have a way to know how many other values are left in the buffer, of course, we can try to check if there is more data in the buffer, but it seems more error-prone and hacky |
@philiphacks see the above, is the payload type something that changed on your side from v1 to v2? |
Hi there, im getting the same error trying to launch a memecoin on stx.city Has there been any sollution for this yet? |
@314159265359879 I doubt it - I don't know what we would have changed since the implementation works with SW wallets; not sure I understand the question :) |
@pete-watters thank you for coordinating the investigation with Zondax's team! any update? |
Hi @lgalabru, I don't have a further update yet. When I spoke to the Zondax team they ran the original data through their simulator and it worked. They then came back to me with these Q's about the data provided:
The error message we see in In @314159265359879 's original report he says
I'm unsure if it's to do with the data being passed as the Zondax team mention Do you have a particular use case you are seeing this error for? Is it with Arkadiko or doing something else? If it's different and you can give me some reproduction steps I am happy to take a look and gather some more information on it to try and solve this |
With stacks app version 0.24.2 I nolonger get the "your ledger device has rejected the payload stateing it is invalid" but I still can't proceed. Now the Ledger device screen goes black. It looks like the whole thing turns off. I see this when opening a vault on arkadiko.finance I tested this with two ledger devices, I thought maybe the device was faulty. It is the same on both. As soon as I click "confirm" in the pop-over it just remains on loading and the device turns off: |
Thanks for testing this @314159265359879 . This seems like a different issue but as you said it also happens with Xverse it could be a new problem with the Zondax app. |
Has anyone investigated this issue? We need to adhere to the specification's recommendations, which involve using tuples for passing arguments. This method is safer than reading arguments one by one until the buffer is exhausted, as it prevents the risk of reading invalid bytes when the number of arguments is unknown. Unless this is fixed in the wallets currently in use, this error will continue to occur. In my previous message, I analyzed the passed payload and noticed that the arguments are not being passed as a Clarity tuple:
Instead, they are concatenated directly as:
This is error-prone because our parser expects arguments in tuple format, as recommended by the specification. We recommend updating the wallets or clients to pass arguments using tuples to prevent this issue from recurring. |
Trying to make sense of this thread. This issue discusses problems with both @philiphacks mentions in #5115 (comment) that he found a similar error when signing a message. Are we certain these are related, or is it just a similar error? @neithanmo In #5115 (comment) you share a Stacks transaction payload, and then cite the SIP-018 structured message SIP. If I'm not mistaken, this SIP has no relevance to transaction signing? |
Thanks @kyranjamie, I did some more investigating also checking differences with different Stacks app versions. It looks like they are unrelated. Just throwing the same general error. Way forward: With this issue lets focus only on resolving the issue with Arkadiko Vaults. For which we have reproduction steps in the first post. Sidenotes This comment relates to this issue #4683 and is partially resolved on the live version and completely resolved with version 0.24.4+ (not yet live). |
Retested opening a vault on Arkadiko (reproduction steps in first post) this still fails the same way) with 0.24.5 of the Stacks App (sideloaded on Ledger Nano S+). According to this comment by neithanmo #5115 (comment) the wallet is passing the arguments concatenated rather than as a tuple which is causing the issue. @kyranjamie can you create a Leather build in which we do pass the arguments as a tuple, so we can test if that resolves the issue? |
I'm not particularly well versed Stacks tx construction, would love input from others cc/ @janniks @jbencin From what I can see, Contract call Where is it stated that contract call args must be a tuple? Also |
Reproduction steps
A user reported that the same thing happens with other vault operations.
It wasn't an issue prior to this vault v2 merge on Arkadiko's side arkadiko-dao/arkadiko#557
The text was updated successfully, but these errors were encountered: