Skip to content

Tbtc Maintainer: Added the maintainer command#3374

Merged
pdyraga merged 36 commits intomainfrom
relay-maintainer-command
Nov 29, 2022
Merged

Tbtc Maintainer: Added the maintainer command#3374
pdyraga merged 36 commits intomainfrom
relay-maintainer-command

Conversation

@tomaszslabon
Copy link
Copy Markdown
Contributor

@tomaszslabon tomaszslabon commented Oct 24, 2022

This PR adds a command for launching the maintainer for Bitcoin difficulty.
The relay maintainer can be launched using the command:

KEEP_ETHEREUM_PASSWORD="password" ./keep-client maintainer  --bitcoinDifficulty  --config /Users/tslabon/projekt/keep-core/configs/maintainer.toml --developer

In the future we can add other maintainer tasks and launch the maintainer, e.g.:

KEEP_ETHEREUM_PASSWORD="password" ./keep-client maintainer  --bitcoinDifficulty --spv  --config /Users/tslabon/projekt/keep-core/configs/maintainer.toml --developer

Maintainer uses 3 groups of configuration flags:

  • ethereum (url, keyFile, etc.)
  • electrum (url)
  • maintainer (bitcoinDifficulty, spv, etc.)

All the options can be provided on the command line or using a configuration file or a mix of them.
Here is an example configuration toml file:

[ethereum]
  URL = "ws://127.0.0.1:8546"
  KeyFile = "/path/to/ethereum/data/keystore/UTC--2022-10-03T10-45-23.667111000Z--7ca4c3c109851221b6560c7d4f3917db6d056977"

[bitcoin.electrum]
  URL = "https://blockstream.info/api/"

[tbtc.maintainer]
 BitcoinDifficulty = true

Tested locally.

@tomaszslabon tomaszslabon requested review from lukasz-zimnoch, nkuba and pdyraga and removed request for lukasz-zimnoch and pdyraga October 24, 2022 12:30
@tomaszslabon tomaszslabon force-pushed the relay-maintainer-command branch from 0fee3de to 74590ac Compare October 24, 2022 12:35
@lukasz-zimnoch
Copy link
Copy Markdown
Member

I'll open a PR with the bitcoin groundwork soon. Please hold off with the merge of this PR until so.

@lukasz-zimnoch
Copy link
Copy Markdown
Member

The PR with the Bitcoin toys is ready: #3384

Comment thread cmd/relay.go Outdated
Comment thread pkg/maintainer/relay.go Outdated
Comment thread pkg/maintainer/relay.go Outdated
@tomaszslabon tomaszslabon force-pushed the relay-maintainer-command branch from 74590ac to 9ca81cc Compare October 27, 2022 12:57
@tomaszslabon tomaszslabon changed the base branch from main to bitcoin-groundwork October 27, 2022 12:57
@tomaszslabon tomaszslabon force-pushed the relay-maintainer-command branch from 9ca81cc to 5e40146 Compare October 27, 2022 13:00
@tomaszslabon tomaszslabon force-pushed the relay-maintainer-command branch from f2f0985 to dcecec3 Compare October 27, 2022 13:33
@tomaszslabon tomaszslabon self-assigned this Oct 27, 2022
@tomaszslabon tomaszslabon linked an issue Oct 27, 2022 that may be closed by this pull request
@tomaszslabon tomaszslabon force-pushed the relay-maintainer-command branch from 80d5bcb to 87762b8 Compare October 28, 2022 11:25
@tomaszslabon tomaszslabon force-pushed the relay-maintainer-command branch from f95e7f0 to 8ff5362 Compare October 28, 2022 11:48
@tomaszslabon tomaszslabon marked this pull request as ready for review October 28, 2022 12:48
@tomaszslabon tomaszslabon force-pushed the relay-maintainer-command branch 2 times, most recently from 1edb3bf to ed5ee62 Compare October 31, 2022 14:56
Comment thread cmd/cmd.go Outdated
Comment thread cmd/flags.go Outdated
Comment thread cmd/flags.go Outdated
Comment thread cmd/flags.go Outdated
Comment thread cmd/maintainer.go Outdated
Comment thread pkg/maintainer/bitcoindifficulty.go
@tomaszslabon tomaszslabon changed the title Relay Maintainer: Added the relay command Tbtc Maintainer: Added the maintainer command Nov 21, 2022
Comment thread cmd/flags.go
Comment thread cmd/flags_test.go
Comment thread cmd/tbtc_maintainer.go Outdated
Comment thread config/category.go
Comment thread test/config.toml Outdated
Comment thread pkg/tbtc/maintainer/bitcoin_difficulty.go Outdated
@pdyraga
Copy link
Copy Markdown
Member

pdyraga commented Nov 22, 2022

I tried to test it starting the maintainer with KEEP_ETHEREUM_PASSWORD="password" ./keep-client --config configs/config.1.toml maintainer but nothing is printed to the console and it looks like if the process hanged. What am I doing wrong?

@tomaszslabon
Copy link
Copy Markdown
Contributor Author

tomaszslabon commented Nov 22, 2022

I tried to test it starting the maintainer with KEEP_ETHEREUM_PASSWORD="password" ./keep-client --config configs/config.1.toml maintainer but nothing is printed to the console and it looks like if the process hanged. What am I doing wrong?

You need to provide the --bitcoinDifficulty flag:

KEEP_ETHEREUM_PASSWORD="password" ./keep-client maintainer  --bitcoinDifficulty  --config /Users/tslabon/projekt/keep-core/configs/maintainer.toml 

Alternatively, you can add:

[tbtc.maintainer]
 BitcoinDifficulty = true

to your config file.

@pdyraga
Copy link
Copy Markdown
Member

pdyraga commented Nov 22, 2022

I was under the impression that we start all maintainers by default?

@tomaszslabon
Copy link
Copy Markdown
Contributor Author

tomaszslabon commented Nov 22, 2022

I was under the impression that we start all maintainers by default?

I wanted to leave a TODO for that (once we have more than one maintainer). But now I added launching all maintainer if not option specified in c87ccff.

Comment on lines +3 to +4
// BitcoinConfig defines the configuration for Bitcoin.
type BitcoinConfig struct {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about naming it just Config? We would have electrum.Config living in pkg/bitcoin/electrum referenced from [bitcoin.electrum] section. I think it all makes sense.

Copy link
Copy Markdown
Contributor Author

@tomaszslabon tomaszslabon Nov 23, 2022

Choose a reason for hiding this comment

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

There is already such a structure in the electrum package and it contains config for electrum (it was added by Kuba and I copied it). It's the structure used by BitcoinConfig. So I cannot rename BitcoinConfig to Config unless I change the name of that structure as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, I think we should have just one config (cc @nkuba) structure named Config:

package electrum

// Config defines the configuration for the Electrum Bitcoin client.
type Config struct {
  // URL to the Electrum server in format: `hostname:port`.
  URL string
}

Based on this conversation, we agreed that we are not configuring Bitcoin but we are configuring Electrum Bitcoin client. The configuration is in [bitcon.electrum] in the config and the structure is in pkg/bitcoin/electrum/config.go.

Copy link
Copy Markdown
Contributor Author

@tomaszslabon tomaszslabon Nov 23, 2022

Choose a reason for hiding this comment

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

If we want to use a nested structure in the config file:

[bitcoin.electrum]
URL = "url.to.electrum:1111"

then the corresponding structure in Golang should also be nested:

type BitcoinConfig struct {
 Electrum Config
}

type Config struct {
 	URL string
}

If we don't use a nested structure, but just a simple non-nested Config structure, then the Viper package that we use for mapping config file to the Golang structure will be unable to read the configuration properly.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I proposed an update in 40ff528

@pdyraga pdyraga merged commit b982ee3 into main Nov 29, 2022
@pdyraga pdyraga deleted the relay-maintainer-command branch November 29, 2022 13:54
@pdyraga pdyraga modified the milestones: v2.0.0-m3, v2.0.0-m2 Jan 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maintainer code for updating relay difficulty

4 participants