Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

feat: implement the broadcaster #115

Merged
merged 7 commits into from
Feb 14, 2023
Merged

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented Feb 9, 2023

Overview

Closes #114

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

@rach-id rach-id added the p2p p2p network related label Feb 9, 2023
@rach-id rach-id requested a review from rahulghangas February 9, 2023 16:03
@rach-id rach-id requested a review from evan-forbes as a code owner February 9, 2023 16:03
@rach-id rach-id self-assigned this Feb 9, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #115 (684ef0f) into main (d3ccfbd) will decrease coverage by 18.82%.
The diff coverage is 75.00%.

@@             Coverage Diff             @@
##             main     #115       +/-   ##
===========================================
- Coverage   61.44%   42.62%   -18.82%     
===========================================
  Files          11       14        +3     
  Lines         625      929      +304     
===========================================
+ Hits          384      396       +12     
- Misses        194      486      +292     
  Partials       47       47               
Impacted Files Coverage Δ
orchestrator/orchestrator.go 0.00% <0.00%> (ø)
orchestrator/broadcaster.go 100.00% <100.00%> (ø)
orchestrator/retrier.go 0.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@rahulghangas
Copy link
Contributor

Apologies for the delayed review on this, will be done today

evan-forbes
evan-forbes previously approved these changes Feb 14, 2023
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

nice! 👍 👍

}

// Note: broadcaster implementation will be done after defining the P2P interfaces.
func (b Broadcaster) BroadcastDataCommitmentConfirm(ctx context.Context, nonce uint64, confirm types.DataCommitmentConfirm) error {
Copy link
Member

Choose a reason for hiding this comment

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

minor optional nit on naming, perhaps "provide", or "put" would be better here

Copy link
Member Author

Choose a reason for hiding this comment

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


// create a test DataCommitmentConfirm
expectedConfirm := types.DataCommitmentConfirm{
EthAddress: "celes1qktu8009djs6uym9uwj84ead24exkezsaqrmn5",
Copy link
Contributor

Choose a reason for hiding this comment

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

EthAddress?

Copy link
Member Author

Choose a reason for hiding this comment

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

6dea8bc
I lost it when I was rebasing, good catch 👍 👍

@rach-id rach-id merged commit fee6adf into celestiaorg:main Feb 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
p2p p2p network related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement the P2P broadcaster
4 participants