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

Allow to use eth private key that starts with 0x #1887

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changelog/1887.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Allow to use eth private key that starts with 0x
38 changes: 37 additions & 1 deletion playwright/tests/paraTimes.spec.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { test, expect } from '@playwright/test'
import { privateKey, privateKeyAddress } from '../../src/utils/__fixtures__/test-inputs'
import { privateKey, privateKeyAddress, ethAccount } from '../../src/utils/__fixtures__/test-inputs'
import { fillPrivateKeyWithoutPassword } from '../utils/fillPrivateKey'
import { warnSlowApi } from '../utils/warnSlowApi'
import { mockApi } from '../utils/mockApi'
Expand Down Expand Up @@ -33,4 +33,40 @@ test.describe('ParaTimes', () => {
await page.getByRole('button', { name: /Next/i }).click()
await expect(page.getByPlaceholder('0x...')).toHaveValue('')
})

test('should validate eth private key', async ({ page }) => {
const validKey = ethAccount.privateKey
const validKeyWithPrefix = `0x${validKey}`
const invalidKey = validKey.replace('c', 'g')
const invalidKeyWithPrefix = `0x${invalidKey}`

async function testPrivateKeyValidation(key, expected) {
await page.getByPlaceholder('Enter Ethereum-compatible private key').fill(key)
await page.getByRole('button', { name: 'Next' }).click()
await expect(page.getByText(expected)).toBeVisible()
}

await page.goto('/open-wallet/private-key')
await fillPrivateKeyWithoutPassword(page, {
privateKey: privateKey,
privateKeyAddress: privateKeyAddress,
persistenceCheckboxChecked: false,
persistenceCheckboxDisabled: false,
})
await page.getByTestId('nav-paratime').click()
await page.getByRole('button', { name: /Withdraw/i }).click()
await page.getByRole('button', { name: 'Select a ParaTime' }).click()
await expect(page.getByRole('listbox')).toBeVisible()
await page.getByRole('listbox').locator('button', { hasText: 'Sapphire' }).click()
await page.getByRole('button', { name: 'Next' }).click()
await page.getByPlaceholder(privateKeyAddress).fill(privateKeyAddress)
// valid eth private keys
await testPrivateKeyValidation(validKey, /enter the amount/)
await page.getByRole('button', { name: 'Back' }).click()
await testPrivateKeyValidation(validKeyWithPrefix, /enter the amount/)
await page.getByRole('button', { name: 'Back' }).click()
// invalid eth private keys
await testPrivateKeyValidation(invalidKey, /private key is invalid/)
await testPrivateKeyValidation(invalidKeyWithPrefix, /private key is invalid/)
})
})
15 changes: 11 additions & 4 deletions src/app/lib/eth-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
import * as oasis from '@oasisprotocol/client'
import * as oasisRT from '@oasisprotocol/client-rt'
import { bytesToHex, isValidPrivate, privateToAddress, toChecksumAddress } from '@ethereumjs/util'
export { isValidAddress as isValidEthAddress } from '@ethereumjs/util'
import {
bytesToHex,
isValidPrivate,
privateToAddress,
toChecksumAddress,
stripHexPrefix,
} from '@ethereumjs/util'
export { isValidAddress as isValidEthAddress, stripHexPrefix } from '@ethereumjs/util'

export const hexToBuffer = (value: string): Buffer => Buffer.from(value, 'hex')
export const isValidEthPrivateKey = (ethPrivateKey: string): boolean => {
try {
return isValidPrivate(hexToBuffer(ethPrivateKey))
return isValidPrivate(hexToBuffer(stripHexPrefix(ethPrivateKey)))
} catch {
return false
}
}
export const isValidEthPrivateKeyLength = (ethPrivateKey: string) => ethPrivateKey.length === 64
export const isValidEthPrivateKeyLength = (ethPrivateKey: string) =>
stripHexPrefix(ethPrivateKey).length === 64
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's .replace('0x', '') a few lines below - should make consistent, either stripHexPrefix or replace('0x'

export const privateToEthAddress = (ethPrivateKey: string): string =>
toChecksumAddress(bytesToHex(privateToAddress(hexToBuffer(ethPrivateKey))))

Expand Down
4 changes: 2 additions & 2 deletions src/app/state/evmAccounts/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { PayloadAction } from '@reduxjs/toolkit'
import { createSlice } from 'utils/@reduxjs/toolkit'
import { EvmAccounts } from './types'
import { privateToEthAddress } from '../../lib/eth-helpers'
import { privateToEthAddress, stripHexPrefix } from '../../lib/eth-helpers'

export const initialState: EvmAccounts = {}

Expand All @@ -10,7 +10,7 @@ export const evmAccountsSlice = createSlice({
initialState,
reducers: {
add(state, action: PayloadAction<{ ethPrivateKey: string }>) {
const ethAddress = privateToEthAddress(action.payload.ethPrivateKey)
const ethAddress = privateToEthAddress(stripHexPrefix(action.payload.ethPrivateKey))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like how far this has spread :/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to reduce mistakes, I'd either isolate 0x prefixes more (mz/privKey...lw/privKey)
or add parsing to all functions that consume it (e.g. privateToEthAddress)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is alter PR #1923

state[ethAddress] = {
ethPrivateKey: action.payload.ethPrivateKey,
ethAddress: ethAddress,
Expand Down
6 changes: 3 additions & 3 deletions src/app/state/paratimes/saga.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { call, put, select, takeLatest } from 'typed-redux-saga'
import * as oasis from '@oasisprotocol/client'
import { accounts, token } from '@oasisprotocol/client-rt'
import { getEvmBech32Address, privateToEthAddress } from 'app/lib/eth-helpers'
import { getEvmBech32Address, privateToEthAddress, stripHexPrefix } from 'app/lib/eth-helpers'
import { submitParaTimeTransaction } from 'app/state/transaction/saga'
import { getOasisNic } from 'app/state/network/saga'
import { selectSelectedNetwork } from 'app/state/network/selectors'
Expand Down Expand Up @@ -54,7 +54,7 @@ export function* fetchBalance(oasisAddress: string, paraTime: ParaTime) {
export function* fetchBalanceUsingEthPrivateKey() {
const { transactionForm } = yield* select(selectParaTimes)
try {
const address = privateToEthAddress(transactionForm.ethPrivateKey)
const address = privateToEthAddress(stripHexPrefix(transactionForm.ethPrivateKey))
const oasisAddress = yield* call(getEvmBech32Address, address)
yield* call(fetchBalance, oasisAddress, transactionForm.paraTime!)
} catch (error: any) {
Expand Down Expand Up @@ -85,7 +85,7 @@ export function* submitTransaction() {
}
yield* call(submitParaTimeTransaction, runtime, {
amount: transactionForm.amount,
ethPrivateKey: transactionForm.ethPrivateKey,
ethPrivateKey: stripHexPrefix(transactionForm.ethPrivateKey),
feeAmount: transactionForm.feeAmount || transactionForm.defaultFeeAmount,
feeGas: transactionForm.feeGas,
recipient: transactionForm.recipient,
Expand Down
11 changes: 8 additions & 3 deletions src/utils/__fixtures__/test-inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,11 @@ export const walletExtensionV0PersistedState = {
},
} satisfies WalletExtensionV0State

export const ethAccount = {
address: '0xbA1b346233E5bB5b44f5B4aC6bF224069f427b18',
privateKey: '6593a788d944bb3e25357df140fac5b0e6273f1500a3b37d6513bf9e9807afe2',
}

export const walletExtensionV0UnlockedState = {
account: {
address: 'oasis1qq30ejf9puuc6qnrazmy9dmn7f3gessveum5wnr6',
Expand All @@ -235,9 +240,9 @@ export const walletExtensionV0UnlockedState = {
},
},
evmAccounts: {
'0xbA1b346233E5bB5b44f5B4aC6bF224069f427b18': {
ethAddress: '0xbA1b346233E5bB5b44f5B4aC6bF224069f427b18',
ethPrivateKey: '6593a788d944bb3e25357df140fac5b0e6273f1500a3b37d6513bf9e9807afe2',
[ethAccount.address]: {
ethAddress: ethAccount.address,
ethPrivateKey: ethAccount.privateKey,
},
},
createWallet: { checkbox: false, mnemonic: [] },
Expand Down
Loading