Skip to content
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.

Sign up for GitHub

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

Jump to bottom

service/block, service/header: BlockService broadcasts generated ExtendedHeaders to network #203

Merged
merged 1 commit into from
Nov 18, 2021
Merged

service/block, service/header: BlockService broadcasts generated ExtendedHeaders to network #203

merged 1 commit into from
Nov 18, 2021

Conversation

renaynay
Copy link
Member

@renaynay renaynay commented Nov 16, 2021 •
edited
Loading

block.Service now receives a Broadcaster from the header.Service so that it can Broadcast() new ExtendedHeaders to the network as they are created during the block handling process.

@renaynay renaynay force-pushed the block-service-pipe-headers branch 5 times, most recently from 7644bef to 4128246 Compare November 17, 2021 14:26
@renaynay renaynay changed the title [ DRAFT ] Block service pipe headers service/block, service/header: BlockService broadcasts generated ExtendedHeaders to network Nov 17, 2021
@renaynay renaynay marked this pull request as ready for review November 17, 2021 15:20
Wondertan
Wondertan reviewed Nov 17, 2021
View reviewed changes
node/services/service.go Outdated Show resolved Hide resolved
node/services/service.go Show resolved Hide resolved
adlerjohn
adlerjohn reviewed Nov 18, 2021
View reviewed changes
node/services/service.go Outdated Show resolved Hide resolved
service/block/event_test.go Outdated Show resolved Hide resolved
liamsi
liamsi previously approved these changes Nov 18, 2021
View reviewed changes
Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@liamsi
Copy link
Member

liamsi commented Nov 18, 2021

Is this new?

=== RUN   TestFull_WithRemoteCore
The p2p host is listening on:
*  /ip4/10.1.1.234/tcp/2121/p2p/12D3KooWBCKfb9j5yMQvzQjrzjQMVrpUyszKhoV3zHQB9GPguJc4
*  /ip6/::1/tcp/2121/p2p/12D3KooWBCKfb9j5yMQvzQjrzjQMVrpUyszKhoV3zHQB9GPguJc4
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xb7e780]

goroutine 5508 [running]:
github.com/tendermint/tendermint/libs/pubsub.(*Subscription).Out(0xc00142ba40)
	/home/runner/go/pkg/mod/github.com/celestiaorg/celestia-core@v0.34.14-celestia/libs/pubsub/subscription.go:43
github.com/tendermint/tendermint/rpc/core.Subscribe.func1()
	/home/runner/go/pkg/mod/github.com/celestiaorg/celestia-core@v0.34.14-celestia/rpc/core/events.go:50 +0x9e
created by github.com/tendermint/tendermint/rpc/core.Subscribe
	/home/runner/go/pkg/mod/github.com/celestiaorg/celestia-core@v0.34.14-celestia/rpc/core/events.go:47 +0x485
FAIL	github.com/celestiaorg/celestia-node/node	0.286s

@Wondertan
Copy link
Member

@liamsi, this is the bug in the core

Wondertan
Wondertan reviewed Nov 18, 2021
View reviewed changes
Copy link
Member

@Wondertan Wondertan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve as long as we fix some things later #214

Wondertan
Wondertan previously approved these changes Nov 18, 2021
View reviewed changes
@Wondertan Wondertan merged commit 28b8d3b into celestiaorg:main Nov 18, 2021
@renaynay renaynay deleted the block-service-pipe-headers branch November 18, 2021 18:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adlerjohn adlerjohn adlerjohn left review comments

@Bidon15 Bidon15 Bidon15 approved these changes

@Wondertan Wondertan Wondertan approved these changes

@liamsi liamsi liamsi left review comments

@mattdf mattdf Awaiting requested review from mattdf

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@renaynay @liamsi @Wondertan @Bidon15 @adlerjohn