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

celocli crashes when unlocking funds with ledger ("Ledger device: UNKNOWN_ERROR (0x6b26)") #195

Open
1 task done
pputman-clabs opened this issue Mar 20, 2024 · 10 comments
Assignees

Comments

@pputman-clabs
Copy link
Contributor

pputman-clabs commented Mar 20, 2024

Package

@celo/celocli

Have you ensured that all of these are up to date?

  • the package you are using

What version of the package are you on?

patrick@C3X2C4YGJ0 Downloads % npm list -g 
/Users/patrick/.nvm/versions/node/v18.19.1/lib
 ├── @celo/[email protected] 
 ├── bip39@ 
 ├── [email protected] 
└── [email protected]

What command or function is the bug in?

celocli releasecelo:locked-gold

Operating System

macOS (Apple Silicon)

Describe the bug

When unlocking funds, the OS on ledger seems to crash and restarts. celocli also crashes. See below:

patrick@C3X2C4YGJ0 Downloads % celocli releasegold:locked-gold --contract $CELO_RG_ADDRESS --action unlock  --useLedger --value 100578303998975999900
 ›   Warning: releasegold:locked-gold is not a celocli command.
Did you mean releasecelo:locked-gold? [y/n]: y
Retrieving derivation Paths [ 0 ]
Running Checks:
   ✔  0xFfb9F456d3eef70aB79C58ea9b038513139612c8 is a registered Account 
   ✔  0xFfb9F456d3eef70aB79C58ea9b038513139612c8 is not currently voting on a governance proposal 
   ✔  Account has at least 100.5783039989759999 non-voting Locked Gold over requirement 
All checks passed
maxFeePerGas and maxPriorityFeePerGas are not supported on Ledger yet. Automatically using gasPrice instead.
Sending Transaction: lockedGoldUnlock... failed: Ledger device: UNKNOWN_ERROR (0x6b26)
    TransportStatusError: Ledger device: UNKNOWN_ERROR (0x6b26)
@arthurgousset
Copy link
Contributor

Just to be sure, could you try the command with releasecelo:locked-gold explicitly instead

- % celocli releasegold:locked-gold --contract $CELO_RG_ADDRESS --action unlock  --useLedger --value 100578303998975999900
+ % celocli releasecelo:locked-gold --contract $CELO_RG_ADDRESS --action unlock  --useLedger --value 100578303998975999900

Probably not the cause the of the problem, but good to be extra sure.

@pputman-clabs
Copy link
Contributor Author

pputman-clabs commented Mar 20, 2024

Tried with releasecelo, got the same issue.

patrick@C3X2C4YGJ0 Downloads % celocli releasecelo:locked-gold --contract $CELO_RG_ADDRESS --action unlock  --useLedger --value 100578303998975999900
Retrieving derivation Paths [ 0 ]
Running Checks:
   ✔  0xFfb9F456d3eef70aB79C58ea9b038513139612c8 is a registered Account 
   ✔  0xFfb9F456d3eef70aB79C58ea9b038513139612c8 is not currently voting on a governance proposal 
   ✔  Account has at least 100.5783039989759999 non-voting Locked Gold over requirement 
All checks passed
maxFeePerGas and maxPriorityFeePerGas are not supported on Ledger yet. Automatically using gasPrice instead.
Sending Transaction: lockedGoldUnlock... failed: Ledger device: UNKNOWN_ERROR (0x6b26)
    TransportStatusError: Ledger device: UNKNOWN_ERROR (0x6b26)

@arthurgousset
Copy link
Contributor

arthurgousset commented Mar 21, 2024

@arthurgousset arthurgousset changed the title celocli crashes when unlocking funds with ledger celocli crashes when unlocking funds with ledger ("Ledger device: UNKNOWN_ERROR (0x6b26)") Mar 21, 2024
@arthurgousset
Copy link
Contributor

@nicolasbrugneaux kindly agreed to own this issue. He'll start working on it on Mon, Mar 25.

@aaronmboyd
Copy link
Contributor

aaronmboyd commented May 7, 2024

Just chiming in also getting this error. Bit of a problem for validators, I am trying to change signers and can't use Ledger keys right now. Trying to revert to node 14 / celocli 2.5.1 or 3.0.0 to get it to work. Would love a fix for this!

Is there a confirmed workaround or do I have to derive my private keys and inject them directly to the cli to manage the validator?

@arthurgousset
Copy link
Contributor

Just chiming in also getting this error. Bit of a problem for validators, I am trying to change signers and can't use Ledger keys right now. Trying to revert to node 14 / celocli 2.5.1 or 3.0.0 to get it to work. Would love a fix for this!

Is there a confirmed workaround or do I have to derive my private keys and inject them directly to the cli to manage the validator?

Thanks for chiming in @aaronmboyd 👋 @nicolasbrugneaux has been working on a fix, and has been our owner for all things Ledger. He's the best point of contact for this bug. I'll let him get back to you.

@zviadm
Copy link
Contributor

zviadm commented May 17, 2024

@nicolasbrugneaux @arthurgousset just some notes that might be helpful:
I am pretty sure these Ledger & Celo issues are related to changes that came with the Eth 2.0 (or London fork) changes in transaction format.
Between @celo/connect v4 and v5, transactions changed from using the "gasPrice" stuff to using the new "maxGasFee..." stuff.

Unfortunately, corresponding changes never happened in the Celo Ledger app: https://github.com/celo-org/celo-ledger-spender-app . That stuff still expects old school transactions.

Also the error that it is throwing is I am pretty sure a misnomer. I am fairly certain the Celo-Ledger-Spender-App is jsut throwing this error:
https://github.com/celo-org/celo-ledger-spender-app/blob/70d1c4936091557b6edb71d739499585768457a5/src/main.c#L2693
Because transaction parsing is just failing. Notice the error code: 0x6A80

And then Ledger/hw-app-eth helpfully relabels that error to something that is completely incorrect:
https://github.com/LedgerHQ/ledger-live/blob/afffa7401380a223096107b6ca3c77c438194898/libs/ledgerjs/packages/hw-app-eth/src/Eth.ts#L52

And that is how you get "EthAppPleaseEnableContractData" errors. Even though I am pretty sure errors are way more fundamental, because celo-ledger-spender-app straight up can't decode new "gas price" type of transactions.

@aaronmgdr
Copy link
Member

Hey @zviadm thanks for the report and sorry we didnt respond earlier.

Its true we started defaulting to maxFeePerGas rather than gasPrice, however we quickly added a fix where it defaults back to gasPrice for all transactions signed by ledger. I suggest trying @celo/[email protected]

@nicolasbrugneaux has spent the last few months updating the software that connects ledger to contractkit. And we hired a company (bloo) that has expertise in ledger apps to create an update to celo-ledger-spender. which will support eip1559 and cip64 transactions. It should be available on ledger live in dev mode within a week or two. We believe this new ledger app and the updates the the js packages will solve the remaining issues.

@zviadm
Copy link
Contributor

zviadm commented Jun 11, 2024

Hey @zviadm thanks for the report and sorry we didnt respond earlier.

Its true we started defaulting to maxFeePerGas rather than gasPrice, however we quickly added a fix where it defaults back to gasPrice for all transactions signed by ledger. I suggest trying @celo/[email protected]

@nicolasbrugneaux has spent the last few months updating the software that connects ledger to contractkit. And we hired a company (bloo) that has expertise in ledger apps to create an update to celo-ledger-spender. which will support eip1559 and cip64 transactions. It should be available on ledger live in dev mode within a week or two. We believe this new ledger app and the updates the the js packages will solve the remaining issues.

Have you actually tested it out though? I don't think that change works anyways. While you do set the "gasPrice" and remove "maxGasFee" and "maxPriorityFeePerGas", I think transaction format is still different and it still fails on the Ledger.
At least I can not get @celo/connect v5 to work with Ledger in any way. And considering celocli doesn't work either, I am guessing it isn't actually fixed.

There is also the new "type" parameter that transactions have right? Maybe that is what is making it not backwards compatible still.

EDIT: I tried to debug this further. I think the real issue is that the "feeCurrency" isn't set, so this still doesn't make the transaction a "celo-legacy" type.
I tested this with celocli too. This doesn't work:
celocli lockedgold:withdraw --useLedger ...params...

But this does work:
celocli lockedgold:withdraw --useLedger --gasCurrency=0x765DE816845861e75A25fCA122bb6898B8B1282a ...params...

This can be worked around in the usage of SDK i think by forcefully setting "gasCurrency" even when you want to use CELO.

EDIT#2: Unfortunately I think feeCurrency can't be when you want to use CELO. it has to be null, so it automatically fails to pass this check:

function isCeloLegacy(tx: CeloTx): boolean {

So you can use Ledger with other feeCurrencies, but i think without fixes somewhere in the SDK, it isn't possible to use it with CELO

@aaronmgdr
Copy link
Member

we have release new version of @celo/celocli https://github.com/celo-org/developer-tooling/releases/tag/%40celo%2Fcelocli%405.1.0 It is compatible with the existing celo-ledger-spender and the one that will be released next.

a new version of the celo-ledger-spender is under review LedgerHQ/app-celo-spender#21

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

No branches or pull requests

6 participants