search

harmony-one / bounties

Bounty program is to help the community take part in the development of the Harmony blockchain. It covers from core feature to validator tooling, from dApp development to DeFi integration.
MIT License
59 stars 23 forks source link

`harmonyConfig` v2.0.0 #34

Closed JackyWYX closed 3 years ago

JackyWYX commented 3 years ago

1. Description

harmonyConfig is the data structure used to store toml data for node configuration, which is usually named as harmony.conf persisted on disk when node-operators start the harmony node. (https://github.com/harmony-one/harmony/blob/main/cmd/harmony/config.go#L19)

This config data structure has gone through quite a few small updates since August 2020, from v1.0.0 to v1.0.5. Now some of the data fields in this structure can potentially cause confusion in future releases. On the other side, addressing these confusion is not naive since it might break the backward compatibility of harmony.conf. Thus we will need a systematic approach to update harmonyConfig based on Version.

2. Elaborations

2.1 Requested changes in harmonyConfig

Following fields of harmonyConfig data structure are about DNS sync. DNS Sync is the centralized block synchronization service provided by harmony team. This service will be replace by a more secure and stable decentralized stream sync protocol in the near future. So it would be better to aggregate these fields to one module (e.g. dnsSync) for easy management at future updates:

Network.DNSSyncPort
Network.DNSZone
Network.LegacySyncing
Sync.LegacyClient
Sync.LegacyServer
Sync.LegacyServerPort

At the same time, flags associated with these values shall be handled accordingly with full backward compatibility.

This is the minimum data structure change to harmonyConfig. Any other updates are also welcomed if appropriate.

2.2 Versioned harmonyConfig

The field harmonyConfig.Version can be used to build a version based persistence on harmonyConfig.

2.3 User prompt interface

Note: The prompt shall not pop-out for non-interactive environment like systemd.

3. Acceptance criteria

4. Reward

USD $4,000 equivalent of Harmony ONE token.

gitcoinbot commented 3 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 34834.1182 ONE (3985.86 USD @ $0.11/ONE) attached to it.

gitcoinbot commented 3 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 34834.1182 ONE (3678.06 USD @ $0.1/ONE) has been submitted by:

gitcoinbot commented 3 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 34834.1182 ONE (3430.85 USD @ $0.1/ONE) has been submitted by:

  1. @osadovy

@papiofficial please take a look at the submitted work:

ntsanov commented 3 years ago

I have submitted my work in gitcoin but have not done PR yet. I wanted to explain first what my idea is and how the implementation works and comment on it if necessary.

General

The idea is to migrate any config version that we open to the current. Since the config file is toml, it seemed like an obvious choice to "mutate" the toml tree. In order to do that we define a config migration func decorator:

type ConfigMigrationFunc func(*toml.Tree) *toml.Tree

This allows for easy definition of new versions and a clear history. When adding a new version all that needs to be done is to add to the migrations map ConfigMigrationFunc implementation that migrates the previous version and the corresponding tests. When config file is read, it is automatically migrated to the next version until current version is reached. At the moment any version that does not match 1.0.4 and 2.0.0 is handled as 1.0.4 would have been. This could be changed by adding migrations to previos versions Example - current migration looks like this :

    migrations["1.0.4"] = func(conf *toml.Tree) *toml.Tree {

        zoneField := conf.Get("Network.DNSZone")
        if zone, ok := zoneField.(string); ok {
            conf.Set("DNSSync.Zone", zone)
        }

        portField := conf.Get("Network.DNSSyncPort")
        if port, ok := portField.(int64); ok {
            conf.Set("DNSSync.Port", port)
        }

        syncingField := conf.Get("Network.LegacySyncing")
        if syncing, ok := syncingField.(bool); ok {
            conf.Set("DNSSync.Enabled", syncing)
        }

        clientField := conf.Get("Sync.LegacyClient")
        if client, ok := clientField.(bool); ok {
            conf.Set("DNSSync.Client", client)
        }

        serverField := conf.Get("Sync.LegacyServer")
        if server, ok := serverField.(bool); ok {
            conf.Set("DNSSync.Server", server)
        }

        serverPortField := conf.Get("Sync.LegacyServerPort")
        if serverPort, ok := serverPortField.(int64); ok {
            conf.Set("DNSSync.ServerPort", serverPort)
        }

        conf.Set("Version", "2.0.0")
        return conf
    }

Test file for the migration has been added

Update command

config command with 2 subcommands has been added:

  1. dump - same as dumpconfig
  2. update - updates the input config file to the latest version

User prompt

If the binary is started from terminal, there is a promt for updating the config to the latest version

PS

I have not changed any flag names since I noticed that they do not match exactly with config path even now, and wasn't sure if they should. It would be trivial to deprecate them and add new that match if needed BTW I added ServerPort field to DNSSync but did not find an use for it. Was it removed or I am not looking where I should ?

gitcoinbot commented 3 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 34834.1182 ONE (5077.56 USD @ $0.14/ONE) attached to this issue has been approved & issued to @ntsanov.