-
Notifications
You must be signed in to change notification settings - Fork 58
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
feat: add cip-1854 support to keyagent #1301
Conversation
|
7f5f5ce
to
8e238da
Compare
eaf322a
to
e5bcd3b
Compare
3487275
to
3ae0b1b
Compare
packages/key-management/src/types.ts
Outdated
export interface AccountKeyDerivationPath { | ||
role: KeyRole; | ||
index: number; | ||
purpose: KeyPurpose; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type is used to derive keys from account key that already has purpose
derived
purpose: KeyPurpose; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed on 7c3a32f
const rootPrivateKey = await this.#decryptRootPrivateKey(); | ||
const accountKey = await deriveAccountPrivateKey({ | ||
accountIndex: this.accountIndex, | ||
bip32Ed25519: this.bip32Ed25519, | ||
purpose, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
purpose, | |
purpose: this.purpose, |
@@ -32,6 +32,7 @@ export class KeyAgentTransactionSigner implements TransactionSigner { | |||
tx, | |||
{ | |||
knownAddresses: [], | |||
purpose: this.#account.purpose, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't seem like Bip32Account.purpose
is used anywhere else, so we can probably undo changes in Bip32Account.ts
purpose: this.#account.purpose, | |
purpose: this.#keyAgent.purpose, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed on 7c3a32f
@@ -4,7 +4,7 @@ import { validateFuzzyOptions, withTextFilter } from '../../../src/StakePool/Typ | |||
describe('TypeormStakePoolProvider utils', () => { | |||
describe('validateFuzzyOptions', () => { | |||
it('throws if value is not a valid JSON encoded string', () => | |||
expect(() => validateFuzzyOptions('test')).toThrow('Unexpected token e in JSON at position 1')); | |||
expect(() => validateFuzzyOptions('test')).toThrow(/Unexpected token/)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an odd change for this PR, was this test flaky?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it was. It fails locally as it receives this error message "Unexpected token 'e', "test" is not valid JSON". Maybe updating the expected error message to match the received message would work better than regex? wdyt?
cb6e36d
to
152a69f
Compare
152a69f
to
7c3a32f
Compare
Context
Reason for the change? If an issue exists, reference it here using a keyword
Proposed Solution
Important Changes Introduced