7.0.0 #30
7.0.0 #30
Conversation
* standardize providers * fix build by upgrading ledgerhq packages * fix specs * update signer field * refactoring * changes according to code review
src/hwProvider.ts
Outdated
| const message = new Message({ | ||
| data: Buffer.from(messageToSign.data), | ||
| address: messageToSign.address ?? Address.fromBech32(this._account.address), | ||
| signer: 'ledger', |
There was a problem hiding this comment.
Can be defined in constants.ts.
| let serializedMessageBuffer = Buffer.from(serializedMessage); | ||
| const message = new Message({ | ||
| data: Buffer.from(messageToSign.data), | ||
| address: messageToSign.address ?? Address.fromBech32(this._account.address), |
There was a problem hiding this comment.
Actually, I think we should always override it instead? That is, I think we should ignore messageToSign.address.
There was a problem hiding this comment.
Is more optimistic for page reload:
- login
- save address to localStorage in dApp
- reload
- sign message
- => ledger device could sign but since address is no longer present in hw-provider, a re login is required
| if (!this.hwApp) { | ||
| throw new ErrNotInitialized(); | ||
| } | ||
|
|
||
| await this.setAddressIndex(options.addressIndex); | ||
| const { address } = await this.hwApp.getAddress(0, options.addressIndex, true); | ||
|
|
||
| return address; | ||
| this._account = { |
There was a problem hiding this comment.
In the PR description, there's:
make login return an IProviderAccount object instead of string
Maybe we should document the breaking changes in a bit more detail? E.g. 6x vs. 7x.
There was a problem hiding this comment.
A "migration support" issue on GitHub could be useful.
Examples:
src/hwProvider.spec.ts
Outdated
|
|
||
| assert.equal(signedMessage.message.toString(), "Hello World"); | ||
| assert.equal(signedMessage.address.toString(), "erd1qyu5wthldzr8wx5c9ucg8kjagg0jfs53s8nr3zpz3hypefsdd8ssycr6th"); | ||
| assert.equal(Buffer.from(signedMessage.data).toString(), "Hello World"); |
There was a problem hiding this comment.
The test is somehow legacy (does not bring very much value).
Maybe the preparation of the data to be signed (get message size, concatenation) can be extracted in a function, and unit tested. Optional.
account, with public methodsgetAccountandsetAccountisConnected()returnbooleaninstead ofPromise<boolean>loginreturn anIProviderAccountobject instead ofstringMessage