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

fix(wallet-client): ignores rpc env var settings #5113

Closed
wants to merge 1 commit into from

Conversation

dpc
Copy link
Contributor

@dpc dpc commented Apr 26, 2024

Edit: See note below.

At least in devimint wallet-client will ignore env vars because it's bitcoind, only to switch to defaults from config ... which are also bitcoind.

@dpc dpc requested a review from a team as a code owner April 26, 2024 06:42
@dpc dpc requested a review from m1sterc001guy April 26, 2024 06:42
@dpc
Copy link
Contributor Author

dpc commented Apr 26, 2024

Oh. I see. We set in devimint the Federation to contain:

        .attach_config_gen_params(
            WalletInit::kind(),
            WalletGenParams {
                local: WalletGenParamsLocal {
                    bitcoin_rpc: bitcoin_rpc.clone(),
                },
                consensus: WalletGenParamsConsensus {
                    network,
                    // TODO this is not very elegant, but I'm planning to get rid of it in a next
                    // commit anyway
                    finality_delay,
                    client_default_bitcoin_rpc: default_esplora_server(
                        bitcoin29_to_bitcoin30_network(network),
                    ),
                    fee_consensus: Default::default(),
                },
            },
        );

Because bitcoind is borked.

Bummer, why do we need so many hacks? :D

AFAIK, we could fix that bitcoind backend by just importing address descriptor for every individual address, or something like that.

But also - is client_default_bitcoin_rpc supposed to be a thing, or is it just a hack? Is Federation supposed to tell the client what backend to use by default?

@dpc dpc marked this pull request as draft April 26, 2024 07:02
@elsirion
Copy link
Contributor

But also - is client_default_bitcoin_rpc supposed to be a thing, or is it just a hack? Is Federation supposed to tell the client what backend to use by default?

It's a hack to avoid needing modularized, user-provided config for the client 😭 I'd say ideally integrators should just choose a sensible default or make it configurable.

@elsirion
Copy link
Contributor

Looks stale, issue was opened instead.

@elsirion elsirion closed this May 28, 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.

None yet

2 participants