fix(Wallet) fix DerivationPathInput allow custom partial path

The non-standard derivation path was not allowed to be entered so paths
with custom indexes were not allowed.
Also implemented warning for non-ethereum coin
Added more tests for the new specifications
Other minor improvements and fixes

Closes #10135
This commit is contained in:
Stefan 2023-04-03 21:30:01 +03:00 committed by Stefan Dunca
parent 9f3d3514af
commit 34c04f0af6
6 changed files with 137 additions and 50 deletions

View File

@ -118,13 +118,22 @@ SplitView {
text: devTxtEdit.errorMessage text: devTxtEdit.errorMessage
visible: devTxtEdit.errorMessage.length > 0 visible: devTxtEdit.errorMessage.length > 0
Layout.alignment: Qt.AlignLeft
Layout.fillWidth: true Layout.fillWidth: true
font.pixelSize: 22 font.pixelSize: 22
font.italic: true font.italic: true
color: "red" color: "red"
} }
Label {
text: devTxtEdit.warningMessage
visible: devTxtEdit.warningMessage.length > 0
Layout.fillWidth: true
font.pixelSize: 22
font.italic: true
color: "orange"
}
RowLayout { RowLayout {
Label { text: "Output: " } Label { text: "Output: " }
Label { id: base; text: devTxtEdit.derivationPath } Label { id: base; text: devTxtEdit.derivationPath }
@ -183,13 +192,11 @@ SplitView {
Label { Label {
text: name text: name
Layout.alignment: Qt.AlignLeft
Layout.fillWidth: true Layout.fillWidth: true
} }
Label { Label {
text: derivationPath text: derivationPath
Layout.alignment: Qt.AlignRight
Layout.fillWidth: true Layout.fillWidth: true
} }
} }

View File

@ -21,6 +21,7 @@ Item {
enabledColor: "white" enabledColor: "white"
frozenColor: "black" frozenColor: "black"
errorColor: "red" errorColor: "red"
warningColor: "orange"
} }
} }
@ -33,7 +34,8 @@ Item {
function test_parseRegularBases_data() { function test_parseRegularBases_data() {
return [ return [
{tag: "Ethereum Standard", base: "m/44'/60'/0'/0", expected: ["m/44'/", "60", "'/", "0", "'/", "0"]}, {tag: "Ethereum Standard", base: "m/44'/60'/0'/0", expected: ["m/44'/", "60", "'/", "0", "'/", "0"]},
{tag: "Custom", base: "m/44'/", expected: ["m/44'/"]}, {tag: "Custom", base: "m/44'", expected: ["m/44'"]},
{tag: "Custom with separator", base: "m/44'/", expected: ["m/44'/"]},
{tag: "Ethereum Ledger", base: "m/44'/60'/0'", expected: ["m/44'/", "60", "'/", "0", "'"]}, {tag: "Ethereum Ledger", base: "m/44'/60'/0'", expected: ["m/44'/", "60", "'/", "0", "'"]},
{tag: "Ethereum Ledger Live", base: "m/44'/60'", expected: ["m/44'/", "60", "'"]}, {tag: "Ethereum Ledger Live", base: "m/44'/60'", expected: ["m/44'/", "60", "'"]},
] ]
@ -54,22 +56,32 @@ Item {
{tag: "Ethereum Ledger", derivationPath: "m/44'/60'/0'/1/2", expected: ["m/44'/", "60", "'/", "0", "'/", "1", "/", "2"]}, {tag: "Ethereum Ledger", derivationPath: "m/44'/60'/0'/1/2", expected: ["m/44'/", "60", "'/", "0", "'/", "1", "/", "2"]},
{tag: "Ethereum Ledger Live", derivationPath: "m/44'/60'/1'/2/3", expected: ["m/44'/", "60", "'/", "1", "'/", "2", "/", "3"]}, {tag: "Ethereum Ledger Live", derivationPath: "m/44'/60'/1'/2/3", expected: ["m/44'/", "60", "'/", "1", "'/", "2", "/", "3"]},
{tag: "Empty entries", derivationPath: "m/44'/'/'//", expected: ["m/44'/", "", "'/", "", "'/", "", "/", ""]}, {tag: "Empty entries", derivationPath: "m/44'/'/'//", expected: ["m/44'/", "", "'/", "", "'/", "", "/", ""]},
{tag: "Wrong entries", derivationPath: "m/44'/T<'/.?'/;/wrong", expected: ["m/44'/", "T<", "'/", ".?", "'/", ";", "/", "wrong"]} {tag: "Wrong entries", derivationPath: "m/44'/T<'/.?'/;/wrong", expected: ["m/44'/", "T<", "'/", ".?", "'/", ";", "/", "wrong"]},
{tag: "Custom corner-case coin only", derivationPath: "m/44'/77", expected: ["m/44'/", "77"]},
{tag: "Custom corner-case hardened coin only", derivationPath: "m/44'/12'", expected: ["m/44'/", "12", "'"]},
{tag: "Custom corner-case coin and separator", derivationPath: "m/44'/12'/", expected: ["m/44'/", "12", "'/", ""]},
{tag: "Custom corner-case coin and address", derivationPath: "m/44'/12'/03", expected: ["m/44'/", "12", "'/", "03"]},
{tag: "Custom corner-case coin and hardened address", derivationPath: "m/44'/12'/03'", expected: ["m/44'/", "12", "'/", "03", "'"]},
{tag: "Custom corner-case coin, address and separator", derivationPath: "m/44'/12'/03'/", expected: ["m/44'/", "12", "'/", "03", "'/", ""]},
{tag: "Custom corner-case coin, address and change", derivationPath: "m/44'/12'/03'/83", expected: ["m/44'/", "12", "'/", "03", "'/", "83"]},
{tag: "Custom corner-case coin, address, change and separator", derivationPath: "m/44'/12'/03'/83/", expected: ["m/44'/", "12", "'/", "03", "'/", "83", "/", ""]},
] ]
} }
function test_parseRegularDerivationPath(data) { function test_parseRegularDerivationPath(data) {
let res = controller.parseDerivationPath(data.derivationPath) let res = controller.parseDerivationPath(data.derivationPath)
compare(res.length, data.expected.length) compare(res.length, data.expected.length, `expect same element count for: ${JSON.stringify(res)} vs ${JSON.stringify(data.expected)}`)
for (const i in res) { for (const i in res) {
compare(data.expected[i], res[i].content) compare(data.expected[i], res[i].content, `expect same element for [${i}]: ${JSON.stringify(res)} vs ${JSON.stringify(data.expected)}`)
} }
} }
function test_completeDerivationPath_data() { function test_completeDerivationPath_data() {
return [ return [
{name: "Ethereum", base: "m/44'/60'/0'/0", derivationPath: "m/44'/60'/0'/0/1", expected: ["m/44'/", "60", "'/", "0", "'/", "0", "/", "1"]}, {tag: "Ethereum", base: "m/44'/60'/0'/0", derivationPath: "m/44'/60'/0'/0/1", expected: ["m/44'/", "60", "'/", "0", "'/", "0", "/", "1"]},
{name: "Ending in separator", base: "m/44'/60'/0'/0", derivationPath: "m/44'/60'/0'/0/", expected: ["m/44'/", "60", "'/", "0", "'/", "0", "/", ""]}, {tag: "Ending in separator", base: "m/44'/60'/0'/0", derivationPath: "m/44'/60'/0'/0/", expected: ["m/44'/", "60", "'/", "0", "'/", "0", "/", ""]},
{name: "Custom", base: "m/44'", derivationPath: "m/44'", expected: ["m/44'/", ""]}, {tag: "Custom", base: "m/44'", derivationPath: "m/44'", expected: ["m/44'/", ""]},
{tag: "Ethereum Ledger", base: "m/44'", derivationPath: "m/44'/60'/0/1/2", expected: ["m/44'/", "60", "'/", "0", "'/", "1", "/", "2"]},
{tag: "Broken Ethereum", base: "m/44'", derivationPath: "m/44'/60/0'/0/1/2", expected: ["m/44'/", "60/0", "'/", "0", "/", "1", "/", "2"]},
] ]
} }
@ -84,11 +96,9 @@ Item {
function test_parseRegularWrongDerivationPath_data() { function test_parseRegularWrongDerivationPath_data() {
return [ return [
{name: "Ethereum", derivationPath: "m'/46'/60'/0'/0/1"}, {tag: "Other standard", derivationPath: "m/46'/60'/0'/0/1"},
{name: "Ethereum", derivationPath: "m/44'/60/0'/0/1/2"}, {tag: "Incomplete", derivationPath: "m/44"},
{name: "Ethereum Ledger", derivationPath: "m/44'/60'/0/1/2"}, {tag: "Ethereum", derivationPath: "'/46'/60'/0'/0/1"},
{name: "Incomplete", derivationPath: "m/44"},
{name: "Ethereum", derivationPath: "'/46'/60'/0'/0/1"},
] ]
} }
@ -97,6 +107,35 @@ Item {
compare(res, null) compare(res, null)
} }
function test_validateAllElements_data() {
return [
{tag: "No error", derivationPath: "m/44'/60'/0'/0/1", error: "", warning: ""},
{tag: "No error regression", derivationPath: "m/44'/", error: "", warning: ""},
{tag: "Non-ethereum", derivationPath: "m/44'/83'/0'/0/1", error: "", warning: "Non-Ethereum cointype"},
{tag: "Numbers only", derivationPath: "m/44'/8d3'/0'/0/1", error: "Please enter numbers only", warning: ""},
{tag: "First address index too big", derivationPath: "m/44'/83'/0'/0/101", error: "Account number must be <100", warning: ""},
{tag: "Last address index too big", derivationPath: "m/44'/83'/0'/0/1/2/123", error: "Account number must be <100", warning: ""},
]
}
function test_validateAllElements(data) {
const res = controller.completeDerivationPath("m/44'", data.derivationPath)
var validationResult = controller.validateAllElements(res.elements)
var expectAllOk = true
if(data.error) {
verify(validationResult.error, `expect error message, got "${validationResult.error}"`)
expectAllOk = false
}
if(data.warning) {
verify(validationResult.warning, `expect warning message, got "${validationResult.warning}"`)
expectAllOk = false
}
if(expectAllOk) {
verify(!validationResult.error, `expect no error message, got "${validationResult.error}"`)
verify(!validationResult.warning, `expect no warning message, got "${validationResult.warning}"`)
}
}
Component { Component {
id: testDataElementsComponent id: testDataElementsComponent
Item { Item {

View File

@ -181,7 +181,7 @@ GridLayout {
id: basePathName id: basePathName
Layout.fillWidth: true Layout.fillWidth: true
visible: !errorMessageText.visible visible: !errorMessageText.visible && !warningMessageText.visible
font.pixelSize: Constants.addAccountPopup.labelFontSize2 font.pixelSize: Constants.addAccountPopup.labelFontSize2
color: Theme.palette.baseColor1 color: Theme.palette.baseColor1
@ -198,6 +198,17 @@ GridLayout {
text: derivationPathInput.errorMessage text: derivationPathInput.errorMessage
} }
StatusBaseText {
id: warningMessageText
Layout.fillWidth: true
visible: !!derivationPathInput.warningMessage
font.pixelSize: basePathName.font.pixelSize
color: Theme.palette.warningColor1
text: derivationPathInput.warningMessage
}
} }
AddressDetails { AddressDetails {

View File

@ -23,6 +23,7 @@ Item {
property alias levelsLimit: controller.levelsLimit property alias levelsLimit: controller.levelsLimit
property alias errorMessage: d.errorMessage property alias errorMessage: d.errorMessage
property alias warningMessage: d.warningMessage
property alias input: input property alias input: input
@ -38,7 +39,7 @@ Item {
if(res.errorMessage) { if(res.errorMessage) {
return false return false
} }
d.resetErrorMessage() d.resetMessages()
d.elements = res.elements d.elements = res.elements
d.updateText(d.elements) d.updateText(d.elements)
input.cursorPosition = d.elements[d.elements.length - 1].endIndex input.cursorPosition = d.elements[d.elements.length - 1].endIndex
@ -57,8 +58,9 @@ Item {
property int cursorPositionToRestore: -1 property int cursorPositionToRestore: -1
property string errorMessage: "" property string errorMessage: ""
property string warningMessage: ""
function resetErrorMessage() { errorMessage = "" } function resetMessages() { errorMessage = ""; warningMessage = "" }
readonly property bool selectionIsActive: Math.abs(input.selectionEnd - input.selectionStart) > 0 readonly property bool selectionIsActive: Math.abs(input.selectionEnd - input.selectionStart) > 0
@ -82,6 +84,7 @@ Item {
enabledColor: root.enabled ? Theme.palette.directColor1 : Theme.palette.baseColor1 enabledColor: root.enabled ? Theme.palette.directColor1 : Theme.palette.baseColor1
frozenColor: Theme.palette.getColor('grey5') frozenColor: Theme.palette.getColor('grey5')
errorColor: Theme.palette.dangerColor1 errorColor: Theme.palette.dangerColor1
warningColor: Theme.palette.warningColor1
} }
StatusBaseInput { StatusBaseInput {
@ -213,15 +216,16 @@ Item {
return return
} }
const currentText = edit.getText(0, text.length) const currentText = edit.getText(0, text.length)
const errorText = controller.validateAllElements(d.elements) const validationRes = controller.validateAllElements(d.elements)
if(d.cursorPositionToRestore >= 0) { if(d.cursorPositionToRestore >= 0) {
input.cursorPosition = d.cursorPositionToRestore input.cursorPosition = d.cursorPositionToRestore
d.cursorPositionToRestore = -1 d.cursorPositionToRestore = -1
} }
d.errorMessage = errorText d.errorMessage = validationRes.error
if(errorText.length > 0 || !d.elements.slice(0, -1).every(obj => obj.content.length > 0)) { d.warningMessage = validationRes.warning
if(d.errorMessage.length > 0 || !d.elements.slice(0, -1).every(obj => obj.content.length > 0)) {
d.currentDerivationPath = "" d.currentDerivationPath = ""
} else { } else {
d.currentDerivationPath = currentText d.currentDerivationPath = currentText

View File

@ -9,21 +9,25 @@ Item {
required property color enabledColor required property color enabledColor
required property color frozenColor required property color frozenColor
required property color errorColor required property color errorColor
required property color warningColor
/// Don't allow inserting more than \c levelsLimit Number elements /// Don't allow inserting more than \c levelsLimit Number elements
property int levelsLimit: 0 property int levelsLimit: 0
readonly property string inputError: qsTr("Please enter numbers only") readonly property string inputError: qsTr("Please enter numbers only")
readonly property string tooBigError: qsTr("Account number must be <100") readonly property string tooBigError: qsTr("Account number must be <100")
readonly property string nonEthCoinWarning: qsTr("Non-Ethereum cointype")
QtObject { QtObject {
id: d id: d
// d flag and named capture groups not supported in Qt 5.15. Use multiple regexes instead // d flag and named capture groups not supported in Qt 5.15. Use multiple regexes instead
readonly property var derivationPathRegex: /^m\/44[|'](?:\/(?<coin_type>.*?)[|'])?(?:\/(?<account>.*?)[|'])?(?:\/(?<change>.*?))?((?:\/.*?)?)$/ readonly property var derivationPathRegex: /^m\/44[|']\/?(?:(?<coin_type_h>.*?)[|'])?(?<coin_type_s>.*?)(?:\/(?<account>.*?)[|']?)?(?:\/(?<change>.*?))?((?:\/.*?)?)$/
// Workaround to missing capture group offsets in Qt 5.15 // The expected characters before each group. Workaround to missing capture group offsets in Qt 5.15
readonly property var offsets: [6, 2, 2] readonly property var offsets: [6, 0, 2, 2]
readonly property int addressIndexStart: 3 readonly property int addressIndexStart: 3
readonly property int ethereumCoinType: 60
// Reference derivation path used to normalize separators (hardened or not). The last separator will be used // Reference derivation path used to normalize separators (hardened or not). The last separator will be used
property var referenceElements: [] property var referenceElements: []
@ -53,24 +57,34 @@ Item {
var elements = [] var elements = []
var groupIndex = 0 var groupIndex = 0
var currentIndex = 0 var currentIndex = 0
var nextIndex = d.offsets[groupIndex]
elements.push(d.createElement(derivationPath.slice(currentIndex, nextIndex), currentIndex, nextIndex, Element.ContentType.Base))
// Extract the captured groups // Extract the captured groups
const coin_type = matches[1] const coin_type_hardened = matches[1]
const account = matches[2] const coin_type_simple = matches[2]
const change = matches[3] const coin_type = coin_type_hardened || coin_type_simple
const addressIndexes = matches[4] != null && matches[4].length > 0 ? matches[4].substring(1) : ""; // remove the leading slash const account = matches[3]
if (coin_type != null) { const change = matches[4]
const addressIndexes = matches[5] ? matches[5].substring(1) : ""; // remove the leading slash
var nextIndex = coin_type != null ? d.offsets[groupIndex] : derivationPath.length
elements.push(d.createElement(derivationPath.slice(currentIndex, nextIndex), currentIndex, nextIndex, Element.ContentType.Base))
if (coin_type != null && nextIndex < derivationPath.length) {
currentIndex = nextIndex currentIndex = nextIndex
nextIndex = currentIndex + coin_type.length nextIndex = currentIndex + coin_type.length
elements.push(d.createElement(coin_type, currentIndex, nextIndex, Element.ContentType.Number)) elements.push(d.createElement(coin_type, currentIndex, nextIndex, Element.ContentType.Number))
groupIndex++ groupIndex += 2
if(coin_type_simple && derivationPath.length > nextIndex) {
return null
}
// Standard separator if there is an account otherwise keep the rest
currentIndex = nextIndex currentIndex = nextIndex
nextIndex = currentIndex + d.offsets[groupIndex] nextIndex = (account != null ? currentIndex + d.offsets[groupIndex] : derivationPath.length)
if(currentIndex < nextIndex) {
elements.push(d.createElement(derivationPath.slice(currentIndex, nextIndex), currentIndex, nextIndex, Element.ContentType.Separator)) elements.push(d.createElement(derivationPath.slice(currentIndex, nextIndex), currentIndex, nextIndex, Element.ContentType.Separator))
}
if(account != null) { if(account != null) {
currentIndex = nextIndex currentIndex = nextIndex
@ -79,8 +93,10 @@ Item {
groupIndex++ groupIndex++
currentIndex = nextIndex currentIndex = nextIndex
nextIndex = currentIndex + d.offsets[groupIndex] nextIndex = (change != null ? currentIndex + d.offsets[groupIndex] : derivationPath.length)
if(currentIndex < nextIndex) {
elements.push(d.createElement(derivationPath.slice(currentIndex, nextIndex), currentIndex, nextIndex, Element.ContentType.Separator)) elements.push(d.createElement(derivationPath.slice(currentIndex, nextIndex), currentIndex, nextIndex, Element.ContentType.Separator))
}
if(change != null) { if(change != null) {
currentIndex = nextIndex currentIndex = nextIndex
@ -88,7 +104,7 @@ Item {
elements.push(d.createElement(change, currentIndex, nextIndex, Element.ContentType.Number)) elements.push(d.createElement(change, currentIndex, nextIndex, Element.ContentType.Number))
// Check if there are any address indexes // Check if there are any address indexes
if (matches[4] != null && matches[4].length > 0) { if (matches[5] != null && matches[5].length > 0) {
const addressIndexesParts = addressIndexes.split('/') const addressIndexesParts = addressIndexes.split('/')
for (const addressIndex of addressIndexesParts) { for (const addressIndex of addressIndexesParts) {
currentIndex = nextIndex currentIndex = nextIndex
@ -113,16 +129,18 @@ Item {
} }
function generateHtmlFromElements(elements) { function generateHtmlFromElements(elements) {
// d class is disabled and e class is editable // d class is disabled; e class is editable; f class is error; w class is warning
var res = `<style>.d{color:${root.frozenColor};}.e{color:${root.enabledColor};}.f{color:${root.errorColor};}</style>` var res = `<style>.d{color:${root.frozenColor};}.e{color:${root.enabledColor};}.f{color:${root.errorColor};}.w{color:${root.warningColor};}</style>`
const format = (content, cssClass) => `<span class="${cssClass}">${content}</span>` const format = (content, cssClass) => `<span class="${cssClass}">${content}</span>`
var numberLevel = 0 var numberLevel = 0
for(var i = 0; i < elements.length; i++) { for(var i = 0; i < elements.length; i++) {
if (elements[i].isFrozen) { if (elements[i].isFrozen) {
res += format(elements[i].content, "d") res += format(elements[i].content, "d")
} else if(validateElement(elements[i], numberLevel)) { } else if(validateElement(elements[i], elements[i].isNumber() ? numberLevel : -1).error.length > 0) {
res += format(elements[i].content, "f") res += format(elements[i].content, "f")
} else if(validateElement(elements[i], elements[i].isNumber() ? numberLevel : -1).warning.length > 0) {
res += format(elements[i].content, "w")
} else { } else {
res += format(elements[i].content, "e") res += format(elements[i].content, "e")
} }
@ -371,27 +389,35 @@ Item {
updateFollowingIndices(elements, 0) updateFollowingIndices(elements, 0)
} }
/// \return the object {error: "", warning: ""}
function validateAllElements(elements) { function validateAllElements(elements) {
var numberLevel = 0 var numberLevel = 0
var warningMessage = ""
for(var i = 0; i < elements.length; i++) { for(var i = 0; i < elements.length; i++) {
const error = validateElement(elements[i], numberLevel) const res = validateElement(elements[i], elements[i].isNumber() ? numberLevel : -1)
if(error.length > 0) { if(res.error.length > 0) {
return error return res
} else if(res.warning.length > 0 && warningMessage === "") {
warningMessage = res.warning
} }
if(elements[i].isNumber()) { if(elements[i].isNumber()) {
numberLevel++ numberLevel++
} }
} }
return "" return {error: "", warning: warningMessage}
} }
/// Expect -1 if not an number element
/// \return the object {error: "", warning: ""}
function validateElement(element, numberLevel) { function validateElement(element, numberLevel) {
if(!element.validateNumber() && !element.isEmptyNumber()) { if(numberLevel > -1 && !element.validateNumber() && !element.isEmptyNumber()) {
return root.inputError return {error: root.inputError, warning: ""}
} else if(numberLevel == 0 && !element.isEmptyNumber() && element.number() != d.ethereumCoinType) {
return {error: "", warning: root.nonEthCoinWarning}
} else if(numberLevel >= d.addressIndexStart && element.number() >= 100) { } else if(numberLevel >= d.addressIndexStart && element.number() >= 100) {
return root.tooBigError return {error: root.tooBigError, warning: ""}
} }
return "" return {error: "", warning: ""}
} }

View File

@ -50,7 +50,7 @@ QtObject {
function isSimilar(other) { function isSimilar(other) {
return contentType === other.contentType return contentType === other.contentType
&& (contentType === Element.ContentType.Number && (contentType === Element.ContentType.Number
? number() === other.number() ? (number() === other.number()) || (isEmptyNumber() && other.isEmptyNumber())
: (isHardened() === other.isHardened())) : (isHardened() === other.isHardened()))
} }