Skip to content

Remove handling of --local-config#2420

Open
DanShaders wants to merge 2 commits into
ton-blockchain:testnetfrom
DanShaders:remove-unused-code
Open

Remove handling of --local-config#2420
DanShaders wants to merge 2 commits into
ton-blockchain:testnetfrom
DanShaders:remove-unused-code

Conversation

@DanShaders

@DanShaders DanShaders commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator

This is a lot of code for the feature that nobody uses.

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
  • validator-engine.cpp:1921 and dht-server.cpp:448: removing the load_local_config fallback makes first-run config generation fail whenever ${db}/config.json and ${db}/config.json.tmp are absent. --local-config is still accepted, but it is now ignored in this path, and move_as_ok() is called on the failed read. Existing bootstrap flows using --local-config to create the initial engine config will no longer work. Either keep a supported migration/bootstrap path or remove/replace the CLI option and update callers.

  • test-hello-world.cpp:97: the test still decodes config/config2, whose JSON @type is config.local, through generic TL JSON parsing. This PR removes the config.local and related constructors from ton_api.tl, so once TL sources are regenerated this from_json(...).ensure() will fail at runtime. The fixture calls should be removed or changed to a still-valid TL type.

@DanShaders DanShaders force-pushed the remove-unused-code branch from 34afec0 to 91d2ba8 Compare June 5, 2026 05:01
@DanShaders DanShaders changed the title Remove a bunch of unused code Remove handling of --local-config Jun 5, 2026
samsuryajho-maker

This comment was marked as spam.

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
  • P1 validator-engine.cpp:1918 and dht-server.cpp:445: when ${DB_ROOT}/config.json and .tmp are both missing, the code now calls move_as_ok() on the failed read_file() result. Before this commit, that path generated an initial config via load_local_config()/load_empty_local_config(). Fresh DB startup now aborts instead of creating config.json, which breaks the documented first-run flow and likely any clean-node bootstrap. Keep a fallback to load_empty_local_config() or return a clear startup error.

  • P2 network.py:214: tontester still passes --local-config to both validator-engine and dht-server, but this PR removes that option from both binaries. The harness already writes config.json into the --db . directory, so this flag should be removed; otherwise node startup fails during option parsing before tests can run.

@DanShaders DanShaders force-pushed the remove-unused-code branch 2 times, most recently from 49a54c8 to 118bf1d Compare June 5, 2026 14:27
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Commit 118bf1d6:

  • validator-engine.cpp and dht-server.cpp: if ${db}/config.json and ${db}/config.json.tmp are both absent, conf_data_R.move_as_ok() is now reached on an error. Previously this path called load_empty_local_config() and generated the initial config. This breaks fresh-node initialization, including docker/init.sh and the documented first-run flow.

  • validator-engine.cpp: removing the -c option leaves Docker startup broken for existing configs, because docker/init.sh still executes validator-engine -c /var/ton-work/db/config.json .... After this PR that option is unrecognized.

  • test/test-hello-world.cpp: this test still decodes JSON with @type: config.local and dht.config.local, but those TL constructors were removed from ton_api.tl. The generic from_json path should now fail at runtime unless these fixtures are removed or changed.

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Findings for 118bf1d6:

  • validator-engine.cpp and dht-server.cpp: if neither <db>/config.json nor <db>/config.json.tmp exists, conf_data_R is still an error and move_as_ok() will abort/assert. Previously this path generated an initial config. If first-run generation is intentionally removed, this should return a controlled error instead of crashing; otherwise keep the empty-config generation fallback.

  • test-hello-world.cpp: this still decodes config/config2, whose @type is config.local, but the PR removes config.local and dht.config.local from ton_api.tl. The generic from_json(...).ensure() path should now fail; remove or replace these fixtures with a remaining TL type.

Why.. why we had it in the first place... Are we testing C++ or what?
@DanShaders DanShaders force-pushed the remove-unused-code branch from 118bf1d to 85abe98 Compare June 11, 2026 01:42
@github-actions

Copy link
Copy Markdown

Commit fe4194e0: no issues found.

Commit 85abe98c:

  • P1 validator-engine.cpp: fresh Docker startup is broken. docker/init.sh still enters the missing-config initialization path and runs validator-engine --db /var/ton-work/db --ip ... when /var/ton-work/db/config.json does not exist, but this code now calls move_as_ok() after both config reads fail. That aborts before config.json is created, so first-run containers fail at docker/init.sh. Either keep an explicit first-run config generation path for Docker or require/provide config.json before this branch runs.

  • P2 tl/generate/scheme/ton_api.tl: removing the config.local TL constructors leaves test/generate-test-node-config.py generating unusable local config files with @type: config.local, id.config.local, etc. See test/generate-test-node-config.py. That generator should be removed or changed to emit engine.validator.config, otherwise anyone using it gets configs the current binaries can no longer parse.

@DanShaders DanShaders force-pushed the remove-unused-code branch from 85abe98 to 7003d88 Compare June 11, 2026 01:49
@github-actions

Copy link
Copy Markdown
  • docker/init.sh:44: first-run Docker initialization is now broken. This still runs validator-engine ... --db /var/ton-work/db --ip ... when /var/ton-work/db/config.json is missing, but ValidatorEngine::load_config() now immediately tries to read db/config.json and move_as_ok()s the error if neither it nor the temp config exists. So fresh Docker deployments can no longer create the initial config and will exit before the later jq config edits. This script needs to either require/provision config.json up front or use a replacement initializer before removing the old fallback.

@DanShaders DanShaders force-pushed the remove-unused-code branch from 7003d88 to 3817659 Compare June 11, 2026 16:39
@github-actions

Copy link
Copy Markdown
  • validator-engine.cpp and dht-server.cpp: if mandatory config.json and its temp file are both missing, conf_data_R.move_as_ok() hits LOG_CHECK(status_.is_ok()) instead of returning through the existing promise error path. That makes a normal “missing mandatory config” startup case abort/crash rather than logging failed to parse config and exiting cleanly. Consider preserving an explicit if (conf_data_R.is_error()) { promise.set_error(...); return; } after the temp-file fallback.

Now <db_root>/config.json is mandatory. Local config machinery is not
used for any modern way to start the node.
@DanShaders DanShaders force-pushed the remove-unused-code branch from 3817659 to be8a20f Compare June 11, 2026 16:50
@github-actions

Copy link
Copy Markdown

Commit be8a20f4

  • docker/init.sh:41: This now rejects any PUBLIC_IP that is not dotted IPv4 before checking whether config.json already exists. Previously, existing Docker volumes with a hostname or IPv6-style value could still start, and fresh initialization delegated --ip parsing to validator-engine’s init_host_port, which resolves hostnames. This makes those deployments fail at script startup even though removing --local-config doesn’t require that for already-initialized configs. Consider limiting the strict IPv4 requirement to the manual initial config/QUIC update path, or resolving hostnames before writing the numeric TL IP.

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.

2 participants