ledger clear signing upgrades #5966
Conversation
|
@BrodyHughes Have two of these PRs for App and BX related to a Ledger change to Blind Signing warnings. Just need to test that Ledger functionality is still working as expected. We are in the process of submitting metadata for our contracts to remove warnings which might be a follow-up. More details here: https://linear.app/rainbow/issue/APP-1731/ledger-removal-of-blind-signing |
There was a problem hiding this comment.
I am unable to import via ledger on both OS's
Android: crashes after allowing permission
Screen.Recording.2024-08-07.at.2.13.47.PM.mov
iOS: TF = 1.9.34 (34)
Hangs during looking stage (I have the app open, 5 min has passed and still looking)
RPReplay_Final1723054955.MP4
|
I don't have a ledger so can't really review this PR. Going to remove my request for review. |
DanielSinclair
force-pushed
the
@daniel/bump-ledger
branch
2 times, most recently
from
007b0d1
to
bc3c8fa
Compare
DanielSinclair
force-pushed
the
@daniel/bump-ledger
branch
2 times, most recently
from
d07451b
to
97f9afb
Compare
|
Adding my slack comment here so we don't lose it: Ledger testing:
okay i’m not sure if the app is even asking for bluetooth permissions correctly? i am also stuck on the import flow trying to connect to ledger. can confirm:
|
chore: upgrade ledgerhq depens, remove patch The upstream issue that required the patch was corrected. These upgrades will allow for Clear Signing with new Ledger firmware versions chore: upgrade @ledgerhq/hw-app-eth chore: upgrade to new versions
DanielSinclair
force-pushed
the
@daniel/bump-ledger
branch
from
97f9afb
to
606f42b
Compare
| @@ -31,6 +31,50 @@ if (process.env.CI) { | |||
| const rainbowConfig = { | |||
| resolver: { | |||
| blacklistRE, | |||
| resolveRequest: (context, moduleName, platform) => { | |||
There was a problem hiding this comment.
This is required to resolve ledger packages correctly otherwise it won't find them.
| transport => { | ||
| const eth = new AppEth(transport); | ||
| return eth; | ||
| getEthApp(deviceId).then( |
There was a problem hiding this comment.
using .then vs await to keep it compatible with the Signer interface
| return new LedgerSigner(provider, getHdPath({ type: WalletLibraryType.ledger, index: Number(index) }), deviceId); | ||
| } | ||
| } else if (privateKey) { | ||
| return new Wallet(privateKey, provider || web3Provider); |
There was a problem hiding this comment.
This is the line I mentioned before. Now the provider defaults to mainnet instead of being undefined.
| @@ -170,7 +170,7 @@ export default function SendSheet() { | |||
| const [debouncedRecipient] = useDebounce(recipient, 500); | |||
|
|
|||
| const [isValidAddress, setIsValidAddress] = useState(!!recipientOverride); | |||
| const [currentProvider, setCurrentProvider] = useState<StaticJsonRpcProvider | undefined>(); | |||
| const [currentProvider, setCurrentProvider] = useState<StaticJsonRpcProvider | undefined>(getProvider({ chainId: ChainId.mainnet })); | |||
There was a problem hiding this comment.
Provider now defaulting to mainnet. Before it was undefined (breaking ledger mainnet sends)
brunobar79
requested review from
jinchung,
walmat and
derHowie
and removed request for
BrodyHughes,
benisgold and
jinchung
Fixes APP-####
What changed (plus any additional context for devs)
@ledgerhq/hw-app-ethand@ledgerhq/react-native-hw-transport-bleRequires https:/rainbow-me/rainbow-scripts/pull/183
Screen recordings / screenshots
Screen.Recording.2024-10-10.at.12.54.58.PM.mov
What to test