Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DLC flow updated to use blockstream API, added regtest #120

Draft
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

Polybius93
Copy link
Collaborator

No description provided.

@Polybius93 Polybius93 requested a review from m-aboelenein April 24, 2023 16:08
@@ -45,7 +45,11 @@ export default class BitcoinEsploraApiProvider extends ApiInstance implements Bi
constructor(options: EsploraApiProviderOptions) {
const { url, network } = options;
super({
baseURL: url || network == 'Mainnet' ? BTC_BASE_URI_MAINNET : BTC_BASE_URI_TESTNET,
baseURL: url || network === 'Mainnet'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here a sequence of ternaries seems a bit too confusing to read and understand. I would recommend just doing a switch, which will be more clear.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you did that in wallet/index.ts too, so it's probably more consistent too.

Comment on lines +25 to +28
bech32: 'bcrt',
pubKeyHash: 0x6f,
scriptHash: 0xc4,
wif: 0xef,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these are used twice, consider defining them in a const object

@DuskaT021 DuskaT021 added this to the Sprint 33 - May 22 milestone May 8, 2023
@teebszet teebszet marked this pull request as draft January 10, 2024 13:00
dhriaznov pushed a commit that referenced this pull request Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants