One integration #170
One integration #170
Conversation
src/one/onePlugin.js
Outdated
| async importPrivateKey(privateKey: string): { oneKey: string } | void { | ||
| this.connectApi() | ||
|
|
||
| const account = await this.harmonyApi.wallet.addByPrivateKey(privateKey) |
There was a problem hiding this comment.
Need to check for mnemonic and check for validity.
| } | ||
| } | ||
|
|
||
| async createPrivateKey(walletType: string): Promise<Object> { |
There was a problem hiding this comment.
Create and save mnemonic.
src/one/onePlugin.js
Outdated
|
|
||
| async connectApi(walletId?: string): Promise<void> { | ||
| if (!this.harmonyApi.blockchain) { | ||
| this.harmonyApi = new Harmony(URL_MAINNET, { |
There was a problem hiding this comment.
Need ability to connect to multiple different urls.
There was a problem hiding this comment.
Done - added multiple servers support
src/one/onePlugin.js
Outdated
| throw new Error('InvalidPublicAddressError') | ||
| } | ||
|
|
||
| edgeParsedUri.uniqueIdentifier = parsedUri.query.dt || undefined |
There was a problem hiding this comment.
Confirming you are also using dt as a unique identifier and not just copied.
|
|
||
| async encodeUri(obj: EdgeEncodeUri): Promise<string> { | ||
| try { | ||
| this.harmonyApi.crypto.getAddress(obj.publicAddress) |
There was a problem hiding this comment.
Connect to api to ensure it exists.
There was a problem hiding this comment.
connectApi removed. this.harmonyApi initiated in constructor
src/one/oneEngine.js
Outdated
| if (this.walletLocalData.publicKey === tx.to) { | ||
| name = tx.from | ||
| } else { | ||
| name = tx.to |
There was a problem hiding this comment.
Don't assign name. It is meant for human readable text not addresses.
src/one/oneEngine.js
Outdated
| name = tx.to | ||
| } | ||
|
|
||
| let nativeAmount = String(Number(tx.value)) |
There was a problem hiding this comment.
What is tx.value? If its a hex then you can just plug it into bns. Converting to a Number first will cause it to lose precision.
i.e.
let nativeAmount = bns.add('0', tx.value)
src/one/oneEngine.js
Outdated
| currencyCode, | ||
| blockHeight: Number(tx.blockNumber), | ||
| nativeAmount, | ||
| networkFee: String(networkFee), |
There was a problem hiding this comment.
Should already be a string.
| throw new Error('Error: no amount specified') | ||
| } | ||
|
|
||
| if (!nativeAmount || nativeAmount === '0') { |
There was a problem hiding this comment.
Can't send 0 amounts? Ethereum can
There was a problem hiding this comment.
We can send 0 amounts. Removed this validation
src/one/oneEngine.js
Outdated
| const GAS_PRICE = new Unit('1').asGwei().toHex() | ||
| const GAS_LIMIT_BASE = new Unit('21000').asWei().toHex() | ||
|
|
||
| const nativeNetworkFee = String(GAS_PRICE * GAS_LIMIT_BASE) |
There was a problem hiding this comment.
Same code is being executed over and over. Should be pulled out and be a constant value unless you plan on making this dynamic values.
|
@potvik Please run yarn precommit and resolve the remaining flow errors. Once ready we can get this to QA. |
potvik
force-pushed
the
one-integration
branch
from
f477f9f to
010b3e4
Compare
No description provided.