Why dBFT requires NeoGO package #2

Closed
opened 2025-12-28 17:07:37 +00:00 by sami · 16 comments
Owner

Originally created by @im-kulikov on GitHub (Nov 27, 2019).

It's strange to understand that the dBFT package requires NeoGo, which requires dBFT.

I propose to move dependencies into the dBFT package.

That adds the possibility to use the package separately in other projects without NeoGo requirements.

cc @fyrchik @realloc @anatoly-bogatyrev @roman-khimov

Originally created by @im-kulikov on GitHub (Nov 27, 2019). It's strange to understand that the dBFT package requires NeoGo, which requires dBFT. I propose to move dependencies into the dBFT package. That adds the possibility to use the package separately in other projects without NeoGo requirements. cc @fyrchik @realloc @anatoly-bogatyrev @roman-khimov
sami 2025-12-28 17:07:37 +00:00
Author
Owner

@roman-khimov commented on GitHub (Nov 27, 2019):

These are all modules, it makes no sense to shuffle them around. Just use modules from the origin repository that developed them.

That adds the possibility to use the package separately in other projects without NeoGo requirements.

I don't think this potential move would change anything.

@roman-khimov commented on GitHub (Nov 27, 2019): These are all modules, it makes no sense to shuffle them around. Just use modules from the origin repository that developed them. > That adds the possibility to use the package separately in other projects without NeoGo requirements. I don't think this potential move would change anything.
Author
Owner

@im-kulikov commented on GitHub (Nov 27, 2019):

I don't think this potential move would change anything.

That adds a possibility to ignore NeoGO mono repo as a dependency.

The dBFT used two types from NeoGO and only for that it requires it. That sounds strange.

@im-kulikov commented on GitHub (Nov 27, 2019): > I don't think this potential move would change anything. That adds a possibility to ignore NeoGO mono repo as a dependency. The dBFT used two types from NeoGO and only for that it requires it. That sounds strange.
Author
Owner

@im-kulikov commented on GitHub (Nov 27, 2019):

My point is in moving or rewrites that types into dBFT and remove unnecessary dependencies.

@im-kulikov commented on GitHub (Nov 27, 2019): My point is in moving or rewrites that types into dBFT and remove unnecessary dependencies.
Author
Owner

@roman-khimov commented on GitHub (Nov 27, 2019):

That adds a possibility to ignore NeoGO mono repo as a dependency.

And that gives us like... what, really? It's not a linux repository, it's like 18M repository.

The dBFT used two types from NeoGO and only for that it requires it. That sounds strange.

Not so much, it reuses module that it needs to and that's it.

My point is in moving or rewrites that types into dBFT and remove unnecessary dependencies.

My point is that moving code between repositories completely destroys changes history and I just hate that.

And rewriting is like... please, don't. There is a module that implements exactly what you need, hoooray! just use it.

@roman-khimov commented on GitHub (Nov 27, 2019): > That adds a possibility to ignore NeoGO mono repo as a dependency. And that gives us like... what, really? It's not a linux repository, it's like 18M repository. > The dBFT used two types from NeoGO and only for that it requires it. That sounds strange. Not so much, it reuses module that it needs to and that's it. > My point is in moving or rewrites that types into dBFT and remove unnecessary dependencies. My point is that moving code between repositories completely destroys changes history and I just hate that. And rewriting is like... please, don't. There is a module that implements exactly what you need, hoooray! just use it.
Author
Owner

@im-kulikov commented on GitHub (Nov 27, 2019):

Not so much, it reuses module that it needs to and that's it.

Nope. It adds a lot of unnecessary dependencies from NeoGO module.

And that gives us like... what, really? It's not a Linux repository, it's like 18M repository.
Not so much, it reuses module that it needs to and that's it.

For two types I should import 18MB? It sounds ugly.

@im-kulikov commented on GitHub (Nov 27, 2019): > Not so much, it reuses module that it needs to and that's it. Nope. It adds a lot of unnecessary dependencies from NeoGO module. > And that gives us like... what, really? It's not a Linux repository, it's like 18M repository. > Not so much, it reuses module that it needs to and that's it. For two types I should import 18MB? It sounds ugly.
Author
Owner

@im-kulikov commented on GitHub (Nov 27, 2019):

It was assumed that part of the NeoGO package (pkg) will be allocated to separate repositories. Now we are turning NeoGO into a mono repo. What then was the point of putting dBFT as a separate repository, if it still requires NeoGO?

@im-kulikov commented on GitHub (Nov 27, 2019): It was assumed that part of the NeoGO package (pkg) will be allocated to separate repositories. Now we are turning NeoGO into a mono repo. What then was the point of putting dBFT as a separate repository, if it still requires NeoGO?
Author
Owner

@roman-khimov commented on GitHub (Nov 27, 2019):

Nope. It adds a lot of unnecessary dependencies from NeoGO module.

Like io module? That's the only one being added. And to be fair it's only added to satisfy data type, no code should be pulled from it.

For two types I should import 18MB? It sounds ugly.

That's git repository size, I don't think anyone cares about that. But you only really add one tiny module to the executable and that's it.

@roman-khimov commented on GitHub (Nov 27, 2019): > Nope. It adds a lot of unnecessary dependencies from NeoGO module. Like `io` module? That's the only one being added. And to be fair it's only added to satisfy data type, no code should be pulled from it. > For two types I should import 18MB? It sounds ugly. That's git repository size, I don't think anyone cares about that. But you only really add one tiny module to the executable and that's it.
Author
Owner

@roman-khimov commented on GitHub (Nov 27, 2019):

After discussion with @im-kulikov we've settled upon abstracting away this types with interfaces if there is a need to really separate this two repositories.

But looking at how these types are used (throughout the whole repository) it probably won't be that easy.

@roman-khimov commented on GitHub (Nov 27, 2019): After discussion with @im-kulikov we've settled upon abstracting away this types with interfaces if there is a need to really separate this two repositories. But looking at how these types are used (throughout the whole repository) it probably won't be that easy.
Author
Owner

@im-kulikov commented on GitHub (Nov 27, 2019):

Yep. An interface would be a great decision.

@im-kulikov commented on GitHub (Nov 27, 2019): Yep. An interface would be a great decision.
Author
Owner

@fyrchik commented on GitHub (Dec 2, 2019):

What then was the point of putting dBFT as a separate repository, if it still requires NeoGO?

@im-kulikov to be able to use it from both neo-go and neofs.

@fyrchik commented on GitHub (Dec 2, 2019): >What then was the point of putting dBFT as a separate repository, if it still requires NeoGO? @im-kulikov to be able to use it from both `neo-go` and `neofs`.
Author
Owner

@fyrchik commented on GitHub (Dec 2, 2019):

Well, we can probably hide util.Uint* behind something like interface { Equal(..) bool } and require making wrappers in client, because function type can't be a subtype of another type.

But io package from neo-go simplifies marshalling a lot and makes it a lot more readable. It is used mostly for default implementations and we can get rid of it but then require client to implement all possible payloads. I'd rather prefer to be able to use a library that "just works" in simple cases and at the same time is rather flexible to be used in more picky situations.

@fyrchik commented on GitHub (Dec 2, 2019): Well, we can probably hide `util.Uint*` behind something like `interface { Equal(..) bool }` and _require_ making wrappers in client, because function type can't be a subtype of another type. But `io` package from `neo-go` simplifies marshalling a lot and makes it a lot more readable. It is used mostly for default implementations and we can get rid of it but then _require_ client to implement all possible payloads. I'd rather prefer to be able to use a library that "just works" in simple cases and at the same time is rather flexible to be used in more picky situations.
Author
Owner

@im-kulikov commented on GitHub (Dec 2, 2019):

But io package from neo-go simplifies marshalling a lot and makes it a lot more readable.

It's just syntactic sugar and I don't see any problems to use the binary package as is.

@im-kulikov commented on GitHub (Dec 2, 2019): > But io package from neo-go simplifies marshalling a lot and makes it a lot more readable. It's just syntactic sugar and I don't see any problems to use the binary package as is.
Author
Owner

@im-kulikov commented on GitHub (Dec 2, 2019):

Well, we can probably hide util.Uint* behind something like interface { Equal(..) bool } and require making wrappers in client, because function type can't be a subtype of another type.

As we discuss with @roman-khimov, there is ok to use Hasher interface and returns the slice of bytes ([]byte). On the usage side, you can wrap it with an internal type as you want.

@im-kulikov commented on GitHub (Dec 2, 2019): > Well, we can probably hide util.Uint* behind something like interface { Equal(..) bool } and require making wrappers in client, because function type can't be a subtype of another type. As we discuss with @roman-khimov, there is ok to use `Hasher` interface and returns the slice of bytes ([]byte). On the usage side, you can wrap it with an internal type as you want.
Author
Owner

@roman-khimov commented on GitHub (Dec 20, 2023):

I don't think we'll ever untie these.

@roman-khimov commented on GitHub (Dec 20, 2023): I don't think we'll ever untie these.
Author
Owner

@fyfyrchik commented on GitHub (Dec 21, 2023):

I have cleaned modcache and after this I have:

$ go env GOPROXY
direct

$ go test -count=1 ./...
go: downloading github.com/nspcc-dev/neo-go v0.73.1-pre.0.20200303142215-f5a1b928ce09
crypto/hash.go:6:2: github.com/nspcc-dev/neo-go@v0.73.1-pre.0.20200303142215-f5a1b928ce09: invalid pseudo-version: preceding tag (v0.73.1-pre) not found
block/block.go:6:2: github.com/nspcc-dev/neo-go@v0.73.1-pre.0.20200303142215-f5a1b928ce09: invalid pseudo-version: preceding tag (v0.73.1-pre) not found

$ GOPROXY=proxy.golang.org go test -count=1 ./...
?       github.com/nspcc-dev/dbft/simulation    [no test files]
ok      github.com/nspcc-dev/dbft       0.007s
ok      github.com/nspcc-dev/dbft/block 0.003s
ok      github.com/nspcc-dev/dbft/crypto        0.003s
ok      github.com/nspcc-dev/dbft/merkle        0.003s
ok      github.com/nspcc-dev/dbft/payload       0.003s
ok      github.com/nspcc-dev/dbft/timer 1.304s

Makes me appreciate what Go team has done regarding versioning, though.

@fyfyrchik commented on GitHub (Dec 21, 2023): I have cleaned modcache and after this I have: ``` $ go env GOPROXY direct $ go test -count=1 ./... go: downloading github.com/nspcc-dev/neo-go v0.73.1-pre.0.20200303142215-f5a1b928ce09 crypto/hash.go:6:2: github.com/nspcc-dev/neo-go@v0.73.1-pre.0.20200303142215-f5a1b928ce09: invalid pseudo-version: preceding tag (v0.73.1-pre) not found block/block.go:6:2: github.com/nspcc-dev/neo-go@v0.73.1-pre.0.20200303142215-f5a1b928ce09: invalid pseudo-version: preceding tag (v0.73.1-pre) not found $ GOPROXY=proxy.golang.org go test -count=1 ./... ? github.com/nspcc-dev/dbft/simulation [no test files] ok github.com/nspcc-dev/dbft 0.007s ok github.com/nspcc-dev/dbft/block 0.003s ok github.com/nspcc-dev/dbft/crypto 0.003s ok github.com/nspcc-dev/dbft/merkle 0.003s ok github.com/nspcc-dev/dbft/payload 0.003s ok github.com/nspcc-dev/dbft/timer 1.304s ``` Makes me appreciate what Go team has done regarding versioning, though.
Author
Owner

@roman-khimov commented on GitHub (Jan 30, 2024):

This can be useful for Bane, btw. As suggested in #87 the only real problem is util.Uint* types. Coincidentally, it'd be easier for us to use different native types there (avoiding conversion we have currently).

@roman-khimov commented on GitHub (Jan 30, 2024): This can be useful for Bane, btw. As suggested in #87 the only real problem is `util.Uint*` types. Coincidentally, it'd be easier for us to use different native types there (avoiding conversion we have currently).
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
nspcc-dev/dbft#2
No description provided.