backend/frontend: add option to restart in testnet #3157
Conversation
bznein
force-pushed
the
testnetRestart
branch
from
e918d68 to
22f4eb9
Compare
| @@ -1196,7 +1196,7 @@ outer: | |||
| // a user-facing setting. Now we simply use it for migration to decide which coins to add by | |||
| // default. | |||
| func (backend *Backend) persistDefaultAccountConfigs(keystore keystore.Keystore, accountsConfig *config.AccountsConfig) error { | |||
| if backend.arguments.Testing() { | |||
| if backend.Testing() { | |||
There was a problem hiding this comment.
I replaced all occurrences of backend.arguments.Testing() with backend.Testing. Prior to this PR the two calls are the same (backend.Testing() calls backend.arguments.Testing()) but with this new PR there is more logic in backend.Testing() - The alternative would have be to change the field arguments.testing but since everything in arguments is unexported I prefer not to
| } | ||
|
|
||
| // NewBackend creates a new backend with the given arguments. | ||
| func NewBackend(arguments *arguments.Arguments, environment Environment) (*Backend, error) { | ||
| log := logging.Get().WithGroup("backend") | ||
| config, err := config.NewConfig(arguments.AppConfigFilename(), arguments.AccountsConfigFilename()) | ||
| backendConfig, err := config.NewConfig(arguments.AppConfigFilename(), arguments.AccountsConfigFilename()) |
There was a problem hiding this comment.
renaming because I later need to use config as an import name
| @@ -1318,6 +1318,9 @@ | |||
| "customFees": { | |||
| "description": "Lets you enter your own fee when sending." | |||
| }, | |||
| "restartInTestnet": { | |||
| "description": "Use testnet instead of mainnet." | |||
There was a problem hiding this comment.
I'm not good with words :P I'm open to suggestions!
bznein
force-pushed
the
testnetRestart
branch
from
22f4eb9 to
ab39006
Compare
| @@ -0,0 +1,69 @@ | |||
|
|
|||
| /** | |||
| * Copyright 2023-2024 Shift Crypto AG | |||
backend/backend.go
Outdated
| @@ -232,30 +232,34 @@ type Backend struct { | |||
| tstCheckAccountUsed func(accounts.Interface) bool | |||
| // For unit tests, called when `backend.maybeAddHiddenUnusedAccounts()` has run. | |||
| tstMaybeAddHiddenUnusedAccounts func() | |||
|
|
|||
| restartingInTestnet bool | |||
There was a problem hiding this comment.
This is redundant with backend.config.AppConfig().Backend.RestartInTestnet, so I would remove this here to improve clarity. Otherwise I am inclined to think that this value is somehow special or different to what's in the config.
There was a problem hiding this comment.
of course! I completely missed that I could just use that! Thanks.
Edit: I'm not sure that would work though? Because when we start we also want to reset the appConfig flag to false so that it doesn't start in testnet again on the next start, so we need somewhere to store this value, wdyt?
There was a problem hiding this comment.
Right, my bad. For clarity, would this maybe help?
- rename it to
testing bool - set it to
backend.config.AppConfig().Backend.Testing || backend.arguments.Testing()followed by modifying the config to set it back tofalse func (backend *Backend) Testing() bool { return backend.testing }
then the logic would be concentrated and easier to follow I think.
backend/config/config.go
Outdated
| @@ -102,6 +102,9 @@ type Backend struct { | |||
|
|
|||
| // BtcUnit is the unit used to represent Bitcoin amounts. See `coin.BtcUnit` for details. | |||
| BtcUnit coin.BtcUnit `json:"btcUnit"` | |||
|
|
|||
| // RestartInTestnet represents whether the app should launch in testnet on the next start. | |||
| RestartInTestnet bool `json:"restartInTestnet"` | |||
There was a problem hiding this comment.
Maybe a better name is StartInTestnet or just Testnet? The flag is used to turn into testnet when starting, not before. This way it sounds like it would trigger a restart or something like that.
backend/backend.go
Outdated
| // Only override if set in settings. If not, let the existing | ||
| // value (that could come from the --testnet flag) untouched. | ||
| if testnet { | ||
| backend.restartingInTestnet = true | ||
| if err := backendConfig.ModifyAppConfig(func(c *config.AppConfig) error { c.Backend.RestartInTestnet = false; return nil }); err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
This might be better in the Start() method instead of the constructor?
backend/config/config.go
Outdated
| @@ -102,6 +102,9 @@ type Backend struct { | |||
|
|
|||
| // BtcUnit is the unit used to represent Bitcoin amounts. See `coin.BtcUnit` for details. | |||
| BtcUnit coin.BtcUnit `json:"btcUnit"` | |||
|
|
|||
| // RestartInTestnet represents whether the app should launch in testnet on the next start. | |||
There was a problem hiding this comment.
Mention that it resets to false on app launch?
bznein
force-pushed
the
testnetRestart
branch
2 times, most recently
from
b1f5d21 to
d9168e9
Compare
backend/backend.go
Outdated
| @@ -638,6 +644,12 @@ func (backend *Backend) Start() <-chan interface{} { | |||
| backend.configureHistoryExchangeRates() | |||
|
|
|||
| backend.environment.OnAuthSettingChanged(backend.config.AppConfig().Backend.Authentication) | |||
|
|
|||
| if backend.testing { | |||
There was a problem hiding this comment.
tiny nit, but shouldn't this be backendConfig.AppConfig().Backend.StartInTestnet? The config does not need to be touched if testing is true due to the -testnet flag.
bznein
force-pushed
the
testnetRestart
branch
2 times, most recently
from
7a5ff0e to
b4337cd
Compare
bznein
force-pushed
the
testnetRestart
branch
2 times, most recently
from
cff6100 to
68deb0d
Compare
Adds a button in the advanced settings that enables the app to start in testnet automatically on the next start. The setting is only valid for the next start, and it gets reset after that.
bznein
force-pushed
the
testnetRestart
branch
from
68deb0d to
9f085c8
Compare
Adds a button in the advanced settings that enables the app to start in testnet automatically on the next start. The setting is only valid for the next start, and it gets reset after that.
(note: the "enabled" text near the option has been removed, this is a slightly older screenshot)
Note that this PR does not automatically restart the application. This would require a lot more investigation on how to do it properly on all platforms.