-
Notifications
You must be signed in to change notification settings - Fork 23
Add additional methods to BitcoinHelper #58
Conversation
Implemented some additional methods exposed by the Electrum Client which will be needed by the end-to-end test script for testnet.
src/BitcoinHelpers.js
Outdated
* | ||
* @param {string} bitcoinAddress Bitcoin address to check. | ||
* | ||
* @return {Promise<Number>} A promise to the confirmed balance as satoshis. |
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.
in satoshis*
src/BitcoinHelpers.js
Outdated
*/ | ||
estimateFeePerKb: async function(includeWithinBlocks) { | ||
return BitcoinHelpers.withElectrumClient(async electrumClient => { | ||
const feePerKb = await electrumClient.getFeeEstimate(includeWithinBlocks || 1) |
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.
Worth noting, this was always returning 0 in my deposit submitter experiments, which is why I ended up always using the minimum relay fee in the other PR's implementation.
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.
Yep, I have the same. I thought there will be a difference if I use it against BTC testnet but nope. I will remove this method and use the default ones or the minimum relay fee in my test script.
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.
Some teensy notes, but I'm good here. Will probably merge at EOD if there are no updates.
EDIT: Whoops, I see we're still in draft 😬
Thanks for a turbo review @Shadowfiend!. Yep, it's a draft because I'm still testing everything and there is a possibility I will change something. Once I'm done with testing I'll make it ready for review and ping you. |
@Shadowfiend I'm ready here. |
src/BitcoinHelpers.js
Outdated
|
||
return feePerKb | ||
}) | ||
}, |
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.
I'm definitely interested in making this work in the future, btw. Need to see what about our node is making the server side of this fall over.
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.
Let's 🎸
Fix JS linter errors Repair some linter errors introduced by #58
Refs: keep-network/tbtc#703
Implemented some additional methods exposed by the Electrum Client which will be needed by the end-to-end test script for testnet: keep-network/local-setup#20