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.

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: remove gossip code #1495

Merged
merged 1 commit into from
Mar 26, 2024
Merged

feat: remove gossip code #1495

merged 1 commit into from
Mar 26, 2024

Conversation

joshuef
Copy link
Contributor

@joshuef joshuef commented Mar 25, 2024

BREAKING CHANGE: remove gossip as royalties will be collected via DAG

Description

reviewpad:summary

@@ -701,8 +601,8 @@
custom-node-bin-org-name: maidsafe
custom-node-bin-branch-name: main

# TODO: re-enable the below scripts once we have proper way to restart nodes.
# Currently on remote network (not local), the nodes do not handle restart RPC cmd well. They reuse the same
# TODO: re-enable the below scripts once we have proper way to restart nodes.

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
# TODO: re-enable the below scripts once we have proper way to restart nodes.
# Currently on remote network (not local), the nodes do not handle restart RPC cmd well. They reuse the same

# TODO: re-enable the below scripts once we have proper way to restart nodes.

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
@joshuef joshuef marked this pull request as ready for review March 25, 2024 03:43
/// The raw bytes of the received message
#[debug(skip)]
msg: Bytes,
},
/// Transfer notification message received for a public key
TransferNotif {
Copy link
Member

Choose a reason for hiding this comment

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

I think TransferNotif is also related to gossip

@@ -67,26 +64,6 @@ enum Cmd {
#[command(flatten)]
peers: PeersArgs,
},
/// Subscribe to a given Gossipsub topic
Copy link
Member

Choose a reason for hiding this comment

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

TransfersEvents { just above this is also part of gossip

Copy link
Member

@grumbach grumbach left a comment

Choose a reason for hiding this comment

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

LGTM! Spotted a couple of remaining zombie types, but looking good!

@JasonPaulGithub JasonPaulGithub added this pull request to the merge queue Mar 25, 2024
@JasonPaulGithub JasonPaulGithub added the Large Large sized PR label Mar 25, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Mar 25, 2024
@JasonPaulGithub JasonPaulGithub added the Pending on Upcoming Changes Branch is waiting on incoming changes (do not merge) label Mar 25, 2024
@joshuef
Copy link
Contributor Author

joshuef commented Mar 26, 2024

Sorted that. Merged it in,

@joshuef joshuef added this pull request to the merge queue Mar 26, 2024
@joshuef joshuef removed this pull request from the merge queue due to a manual request Mar 26, 2024
@joshuef joshuef enabled auto-merge March 26, 2024 00:30
BREAKING CHANGE: remove gossip as royalties will be collected via DAG
@joshuef joshuef disabled auto-merge March 26, 2024 00:58
@joshuef joshuef merged commit a86a49a into maidsafe:main Mar 26, 2024
50 checks passed
@joshuef joshuef deleted the NoGossip branch March 26, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Large Large sized PR Pending on Upcoming Changes Branch is waiting on incoming changes (do not merge)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants