[GH-846] Added `--bootstrap-url` parameter for simple initialization of archive node by Un1oR · Pull Request #847 · NilFoundation/nil

Un1oR · 2025-04-22T20:52:27Z

This is an alternative approach to the one proposed in #813.

Pros:

No hardcodes
No infrastructure lock-in, works fine locally, with or without relays

Caveats:

Establishing a connection using relays requires additional research, debugging and experimentation with EnableAutoNATv2 and EnableHolePunching in real or near real conditions. To avoid being blocked by this now, I've enabled the WithInfiniteLimits setting on relays, and WithAllowLimitedConn on nodes. This should work "cheap but surly".

nil/cmd/nild/main.go

nil/internal/network/addr_info.go

nil/cmd/nild/main.go

dmtrskv · 2025-04-23T12:37:34Z

nil/services/rpc/types/bootstrap_config.go

@@ -0,0 +1,13 @@
+package types


(Not directly relevant to the PR.)

The package should've been called rpctypes so that one doesn't have to rename it all the time

(actually, I have the same thought about connection_manager which is contra at least two go conventions: no snake_case and a manager cannot be a package; the package should be what the manager manages; I suggest smth like connections there)

dmtrskv · 2025-04-23T12:40:30Z

nil/services/rpc/rawapi/internal/node_builder.go

 func (nb *nodeApiBuilder) WithLocalShardApiRw(shardId types.ShardId, txnpool txnpool.Pool) *nodeApiBuilder {
-	var localShardApi shardApiRw = newLocalShardApiRw(newLocalShardApiRo(shardId, nb.db), txnpool)
+	var bootstrapConfig *rpctypes.BootstrapConfig // = nil — not used in rw API methods
+	var localShardApi shardApiRw = newLocalShardApiRw(newLocalShardApiRo(shardId, nb.db, bootstrapConfig), txnpool)


why not shardApiRw := newLocalShardApiRw(newLocalShardApiRo(shardId, nb.db, nil), txnpool)?

Answered in a neighboring comment. I want to see the interface type explicitly.

To see the explicit interface type for what purpose?
(it's usually shown by an IDE btw)

This is more of a contract for the code below

dmtrskv · 2025-04-23T12:41:10Z

nil/services/rpc/rawapi/internal/node_builder.go

+	shardId types.ShardId,
+	bootstrapConfig *rpctypes.BootstrapConfig,
+) *nodeApiBuilder {
+	var localShardApi shardApiRo = newLocalShardApiRo(shardId, nb.db, bootstrapConfig)


let's stick to :=

Actually, this code was written by me earlier, just changing the formatting here. But I purposely used an explicit type statement for clarity and to express the intent.

I disagree that var localShardApi shardApiRo = newLocalShardApiRo is more clear than localShardApi := newLocalShardApiRo
But up to you

The point is that the "constructor" returns a pointer to the implementation (this may also be controversial, but so far I find it more practical). At the point of assignment, I emphasize that the code should not rely on the details of the implementation, but rather work with the interface.
Yes, this is probably really far-fetched, but I found it convenient.

nil/internal/network/host.go

nil/internal/network/addr_info.go

dmtrskv · 2025-04-23T12:48:16Z

nil/services/nilservice/service.go

 			}()
-			if err = initSyncers(ctx, res.syncers, cfg.AllowDbDrop); err != nil {
+			if err = initSyncers(
+				libp2pnetwork.WithAllowLimitedConn(ctx, "allow working through relay with traffic proxying"),


shouldn't it be done on some lower level?

I think ultimately we need to deal with establishing a direct connection, but as I wrote in the PR description, it's not the most trivial task. So such an obvious "plug" that the eye clings to seems to me preferable to a solution that will pretend to be good.

nil/cmd/nild/main.go

dmtrskv · 2025-04-23T20:34:26Z

nil/services/rpc/rawapi/internal/node_builder.go

+	shardId types.ShardId,
+	bootstrapConfig *rpctypes.BootstrapConfig,
+) *nodeApiBuilder {
+	var localShardApi shardApiRo = newLocalShardApiRo(shardId, nb.db, bootstrapConfig)


I disagree that var localShardApi shardApiRo = newLocalShardApiRo is more clear than localShardApi := newLocalShardApiRo
But up to you

dmtrskv · 2025-04-23T20:35:45Z

nil/services/rpc/rawapi/internal/node_builder.go

 func (nb *nodeApiBuilder) WithLocalShardApiRw(shardId types.ShardId, txnpool txnpool.Pool) *nodeApiBuilder {
-	var localShardApi shardApiRw = newLocalShardApiRw(newLocalShardApiRo(shardId, nb.db), txnpool)
+	var bootstrapConfig *rpctypes.BootstrapConfig // = nil — not used in rw API methods
+	var localShardApi shardApiRw = newLocalShardApiRw(newLocalShardApiRo(shardId, nb.db, bootstrapConfig), txnpool)


To see the explicit interface type for what purpose?
(it's usually shown by an IDE btw)

nil/services/rpc/rawapi/pb/conversion_bootstrap_config.go

…rchive node.

Un1oR requested a review from dmtrskv April 22, 2025 20:52

Un1oR had a problem deploying to prod April 22, 2025 20:52 — with GitHub Actions Failure

Un1oR had a problem deploying to prod April 22, 2025 20:52 — with GitHub Actions Error

Un1oR force-pushed the feature/GH-846-simple-archive-node-bootstrap branch from 9372c0d to aec9e75 Compare April 22, 2025 20:57

Un1oR had a problem deploying to prod April 22, 2025 20:57 — with GitHub Actions Failure

Un1oR had a problem deploying to prod April 22, 2025 20:58 — with GitHub Actions Error

Un1oR changed the title ~~[GH-846] Added bootstrap-url parameter for simple initialization of archive node~~ [GH-846] Added --bootstrap-url parameter for simple initialization of archive node Apr 22, 2025

Un1oR changed the title ~~[GH-846] Added --bootstrap-url parameter for simple initialization of archive node~~ [GH-846] Added bootstrap-url parameter for simple initialization of archive node Apr 22, 2025

Un1oR changed the title ~~[GH-846] Added bootstrap-url parameter for simple initialization of archive node~~ [GH-846] Added --bootstrap-url parameter for simple initialization of archive node Apr 22, 2025

Un1oR force-pushed the feature/GH-846-simple-archive-node-bootstrap branch from aec9e75 to ddf6eb2 Compare April 22, 2025 21:03

Un1oR temporarily deployed to prod April 22, 2025 21:03 — with GitHub Actions Inactive

olegrok reviewed Apr 23, 2025

View reviewed changes

nil/cmd/nild/main.go Outdated Show resolved Hide resolved

nil/cmd/nild/main.go Outdated Show resolved Hide resolved

nil/cmd/nild/main.go Outdated Show resolved Hide resolved

nil/internal/network/addr_info.go Outdated Show resolved Hide resolved

olegrok reviewed Apr 23, 2025

View reviewed changes

nil/cmd/nild/main.go Outdated Show resolved Hide resolved

Un1oR force-pushed the feature/GH-846-simple-archive-node-bootstrap branch from ddf6eb2 to 0b16a1c Compare April 23, 2025 09:22

Un1oR temporarily deployed to prod April 23, 2025 09:22 — with GitHub Actions Inactive

Un1oR temporarily deployed to prod April 23, 2025 11:56 — with GitHub Actions Inactive

dmtrskv reviewed Apr 23, 2025

View reviewed changes

Un1oR force-pushed the feature/GH-846-simple-archive-node-bootstrap branch from 3c259cd to 021a07f Compare April 23, 2025 13:01

Un1oR temporarily deployed to prod April 23, 2025 13:01 — with GitHub Actions Inactive

Un1oR temporarily deployed to prod April 23, 2025 13:02 — with GitHub Actions Inactive

Un1oR had a problem deploying to prod April 23, 2025 15:10 — with GitHub Actions Error

Un1oR temporarily deployed to prod April 23, 2025 15:10 — with GitHub Actions Inactive

Un1oR had a problem deploying to prod April 23, 2025 15:10 — with GitHub Actions Error

Un1oR force-pushed the feature/GH-846-simple-archive-node-bootstrap branch from c9abe69 to 925dfee Compare April 23, 2025 15:15

Un1oR temporarily deployed to prod April 23, 2025 15:16 — with GitHub Actions Inactive

Un1oR requested a review from dmtrskv April 23, 2025 16:36

dmtrskv approved these changes Apr 23, 2025

View reviewed changes

Un1oR added 2 commits April 24, 2025 01:22

[GH-846] Added bootstrap-url parameter for simple initialization of a…

bf5d2a4

…rchive node.

[GH-846] Support relay setting in gen-configs.

c6e4e74

Un1oR force-pushed the feature/GH-846-simple-archive-node-bootstrap branch from 925dfee to c6e4e74 Compare April 23, 2025 21:22

Un1oR temporarily deployed to prod April 23, 2025 21:22 — with GitHub Actions Inactive

Un1oR enabled auto-merge April 23, 2025 21:23

Un1oR added this pull request to the merge queue Apr 23, 2025

Merged via the queue into main with commit 098f6fc Apr 23, 2025
15 of 16 checks passed

Un1oR deleted the feature/GH-846-simple-archive-node-bootstrap branch April 23, 2025 21:48

Conversation

Un1oR commented Apr 22, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants