Javascript sdk migration #617
Conversation
Migrated Account Functions to use Js SDK
Updated bank api calls to use the js sdk
* Validator Api calls made with js sdk, Fixed config types and test * add memo * memo to tables * fixed all errors with updated sdk * included comment for payment handler Co-authored-by: Justin Kim <justinkim943@gmail.com>
|
@tomijaga can you resolve the merge conflict? |
* Validator Api calls made with js sdk, Fixed config types and test * add memo * memo to tables * fixed all errors with updated sdk * included comment for payment handler * add memo (#615) * add memo * memo to tables * IP update Co-authored-by: Bucky <buckyroberts@gmail.com> * 1.0.0-alpha.45 Co-authored-by: Justin Kim <justinkim943@gmail.com> Co-authored-by: Bucky <buckyroberts@gmail.com>
|
Conflicts merged @angle943 |
|
@zinoadidi @tomijaga thanks guys. Please give me about a week to review this. This week I will be very busy moving in to my new home. I will be free starting next week |
| const checkPrivateSigningKey = (publicKey: string, privateKey: string): boolean => { | ||
| try { | ||
| const {publicKeyHex} = getKeyPairFromSigningKeyHex(privateKey); | ||
| return publicKeyHex === publicKey; | ||
| const {accountNumberHex} = new Account(privateKey); | ||
| return accountNumberHex === publicKey; | ||
| } catch (error) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Possibly migrate to using Account.isValidPair static method as it does exactly as this helper does except you need the signing key first and the account number second. Although, the js SDK does not include a try-catch statement which should probably be added. https://github.com/thenewboston-developers/thenewboston-js/blob/development/src/account.ts#L49-L56
Update: sent an Issue and a PR to thenewboston-js repo to include a try-catch statement in Account.isValidPair.
src/renderer/dispatchers/banks.ts
Outdated
| const {data: rawData} = await axios.get<RawBankConfig>(`${address}/config`, {timeout: AXIOS_TIMEOUT_MS}); | ||
| const data = sanitizePortFieldFromRawBankConfig(rawData); | ||
| const rawData = await new Bank(address).getConfig(); | ||
| const data = sanitizePortFieldFromRawBankConfig(rawData as RawBankConfig); |
There was a problem hiding this comment.
Is the as statement even necessary? I think TypeScript should be accepting the SDK's type also as long as no property types conflict and they shouldn't be.
There was a problem hiding this comment.
It's coz the NodeType in the account manager repo is an enum while in the js repo it a type that accepts multiple values ("Bank" | "Primary_Validator" )
There was a problem hiding this comment.
I will make changes in the js repo to fix this problem
* Account.isValidPair method used in repo * correct implementation of Account.isValidPair method * Removed unnecessary use of 'as' keyword
|
@tomijaga some conflicts in package lock. Try to fix locally and make a PR |
|
@angle943 still waiting |
|
Could likely close this now since the new wallet is being made currently for the beta. |
|
Closing in favour of beta |
Resolves: #585
@angle943 I've made all the necessary migrations using the js sdk