-
Notifications
You must be signed in to change notification settings - Fork 21.6k
Description
Rationale
Why should this feature exist?
Currently Ledger only support LegacyTx and I want it to support DynamicTx as the LegacyTx is well, Legacy.
go-ethereum/accounts/usbwallet/ledger.go
Lines 379 to 387 in f59d013
| // Create the correct signer and signature transform based on the chain ID | |
| var signer types.Signer | |
| if chainID == nil { | |
| signer = new(types.HomesteadSigner) | |
| } else { | |
| signer = types.NewEIP155Signer(chainID) | |
| signature[64] -= byte(chainID.Uint64()*2 + 35) | |
| } | |
| signed, err := tx.WithSignature(signer, signature) |
What are the use-cases?
Can use EIP1559 dynamic fee tx
Implementation
Do you have ideas regarding the implementation of this feature?
No - I have been reading the code and trying to see what all changes I will have to make, but I can give it a shot.
From what I understand this block needs to support typed transactions as ethereum app on ledger supports parsing that.
go-ethereum/accounts/usbwallet/ledger.go
Lines 336 to 343 in f59d013
| if chainID == nil { | |
| if txrlp, err = rlp.EncodeToBytes([]interface{}{tx.Nonce(), tx.GasPrice(), tx.Gas(), tx.To(), tx.Value(), tx.Data()}); err != nil { | |
| return common.Address{}, nil, err | |
| } | |
| } else { | |
| if txrlp, err = rlp.EncodeToBytes([]interface{}{tx.Nonce(), tx.GasPrice(), tx.Gas(), tx.To(), tx.Value(), tx.Data(), chainID, big.NewInt(0), big.NewInt(0)}); err != nil { | |
| return common.Address{}, nil, err | |
| } |
I still don't know how to deal with this though
go-ethereum/accounts/usbwallet/ledger.go
Lines 353 to 357 in f59d013
| // Chunk size selection to mitigate an underlying RLP deserialization issue on the ledger app. | |
| // https://github.com/LedgerHQ/app-ethereum/issues/409 | |
| chunk := 255 | |
| for ; len(payload)%chunk <= ledgerEip155Size; chunk-- { | |
| } |
Are you willing to implement this feature?
Yes