-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: start bee node without a connected chain backend #2783
Conversation
f55db4d
to
987f922
Compare
|
da57a9b
to
daa4882
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks good and I have some suggestions/questions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 8 of 15 files at r1.
Reviewable status: 0 of 16 files reviewed, 10 unresolved discussions (waiting on @acud, @aloknerurkar, @istae, @mrekucci, @notanatol, and @ralph-pichler)
pkg/node/chain_stub_test.go, line 14 at r4 (raw file):
Previously, acud (acud) wrote…
any particular reason why these compile-time checks are in a
_test
file?
This is a test of whether a struct implements a certain interface, thus it belongs to tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 16 files reviewed, 10 unresolved discussions (waiting on @acud, @aloknerurkar, @istae, @mrekucci, @notanatol, and @ralph-pichler)
pkg/p2p/libp2p/parser.go, line 29 at r4 (raw file):
Previously, acud (acud) wrote…
not sure why this is needed? can't we somehow tell libp2p to initialize the handshake service in a way that just never verifies the overlay? or in a sense the handshake service would just plug a different noop
parseAck
function that would always approve the ack?
this was merely the solution with the smallest footprint changes wise
the alternative - which is passing down a boolean flag to skip verification seems more cumbersome, but we can entertain it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r9, 1 of 1 files at r10, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @aloknerurkar, @istae, @mrekucci, and @ralph-pichler)
c49e47a
to
25dbddc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 16 files at r1, 2 of 23 files at r11, 24 of 24 files at r13, all commit messages.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @aloknerurkar, @istae, and @ralph-pichler)
pkg/debugapi/postage.go, line 195 at r13 (raw file):
} if resp.Stamps == nil {
Not necessary.
pkg/node/node.go, line 1040 at r13 (raw file):
} func isChainEnabled(o *Options, logger logging.Logger) bool {
Consider attaching it to the Options
struct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 26 of 27 files reviewed, 3 unresolved discussions (waiting on @aloknerurkar, @istae, @mrekucci, and @ralph-pichler)
pkg/debugapi/postage.go, line 195 at r13 (raw file):
Previously, mrekucci wrote…
Not necessary.
removed
Checklist
Description
We want to be able to start a light node with no postage, no overlay verification, no settlements and other chain related functionalities.
This will greatly simplify the onboarding of the
Edina
person since no blockchain provisioning is needed.Motivation and context (Optional)
We need to be able to run bee with minimal knowledge/preparation.
https://hackmd.io/5h4BFSDOSPeWnuUd5OR8EA?view
This change is