Switch to using bignumber.js library #186
Conversation
* node imports * esm maybe? * Prerelease publish v7 alpha.0
* add docs for update method and expose update on solana properly
tatomir-streamflow
requested review from
RolginRoman,
Yolley and
LukaStreamflow
| @@ -44,6 +44,7 @@ | |||
| "@solana/wallet-adapter-base": "0.9.19", | |||
| "@solana/web3.js": "1.70.1", | |||
| "aptos": "1.4.0", | |||
| "bignumber.js": "^9.1.2", | |||
There was a problem hiding this comment.
How do we want to update our public API?
How do we want to version this?
This is just breaking change. We bump major and update README if needed
| ```javascript | ||
| const updateStreamParams: Types.IUpdateData = { | ||
| id: "AAAAyotqTZZMAAAAmsD1JAgksT8NVAAAASfrGB5RAAAA", // Identifier of a stream to update. | ||
| enableAutomaticWithdrawal: true, // [optional], allows to enable AW if it wasn't, disable is not possible |
There was a problem hiding this comment.
Since we are updating this, @Yolley there was some magic combination if I recall, to turn off auto settle?
| return this.depositedAmount | ||
| .minus(this.withdrawnAmount) | ||
| .div(10 ** decimals) | ||
| .toNumber(); |
There was a problem hiding this comment.
I would change API here not to return number.
@Yolley WDYT?
|
|
||
| export const BASE_FEE = 1009900; // Buffer to include usual fees when calculating stream amount | ||
| export const WITHDRAW_AVAILABLE_AMOUNT = new BN("18446744073709551615"); // Magical number to withdraw all available amount from a Contract | ||
| // this magical number needs updating probably |
There was a problem hiding this comment.
Prob, but let's handle it in bug system, not in comment :)
| @@ -1,10 +1,11 @@ | |||
| import BN from "bn.js"; | |||
| import BigNumber from "bignumber.js"; | |||
There was a problem hiding this comment.
Why is BN imported above?
| import BN from "bn.js"; | ||
| import { BigNumber } from "ethers"; | ||
| import BigNumber from "bignumber.js"; | ||
| import { BigNumber as BigNumberEvm } from "ethers"; |
There was a problem hiding this comment.
Nit: I would love to migrate all to BigNumber, and just to have layers that are converting
| @@ -64,6 +64,7 @@ | |||
| "@streamflow/common": "workspace:*", | |||
| "@suiet/wallet-kit": "0.2.22", | |||
| "aptos": "1.4.0", | |||
| "bignumber.js": "^9.1.2", | |||
There was a problem hiding this comment.
Can we remove bn below?
RolginRoman
force-pushed
the
prerelease/v7
branch
from
daaa89a
to
3013c5b
Compare
|
Closing pull request to delete remote branch - having issues pushing new changes after rebasing to v7 so I will reset the "redo" the remote branch |
Overview
We want to switch to using bignumber.js library due to more precision when doing operations like division and multiplication and to have consistency across the codebases (to only use 1 big number library).
Open questions