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: publish to non-subscribed topic #933

Merged
merged 1 commit into from Nov 29, 2023

Conversation

maqi
Copy link
Member

@maqi maqi commented Nov 2, 2023

Description

Summary generated by Reviewpad on 28 Nov 23 16:12 UTC

This pull request adds a new feature where most of the nodes do not subscribe to the "TRANSFER_NOTIFICATION" topic. The patch modifies several files including sn_cli/src/subcommands/wallet.rs, sn_node/src/lib.rs, sn_node/src/node.rs, sn_node/src/put_validation.rs, sn_node/tests/nodes_rewards.rs, and sn_node_rpc_client/src/main.rs. The changes include renaming the constant TRANSFER_NOTIF_TOPIC to ROYALTY_TRANSFER_NOTIF_TOPIC, updating the subscription to the new topic in the listen_notifs_and_deposit function, and handling messages for the new topic in the process_event function. Additionally, the patch introduces a new constant FORWARDER_CHOOSING_FACTOR which determines the number of nodes acting as forwarders for the subscription.

@reviewpad reviewpad bot added Small Pull request is small waiting-for-review labels Nov 2, 2023
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch from 7789f9b to 50ba226 Compare November 3, 2023 10:54
sn_node/tests/nodes_rewards.rs Fixed Show fixed Hide fixed
sn_node/tests/nodes_rewards.rs Fixed Show fixed Hide fixed
sn_node/tests/nodes_rewards.rs Fixed Show fixed Hide fixed
sn_node/tests/nodes_rewards.rs Fixed Show fixed Hide fixed
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch 2 times, most recently from 8e74013 to 7fba5de Compare November 3, 2023 12:02
sn_node/tests/nodes_rewards.rs Fixed Show fixed Hide fixed
sn_node/tests/nodes_rewards.rs Fixed Show fixed Hide fixed
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch from 7fba5de to 4831fb5 Compare November 7, 2023 11:07
@reviewpad reviewpad bot added Medium Medium sized PR and removed Small Pull request is small labels Nov 7, 2023
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch 2 times, most recently from ccfdd4c to e381bb6 Compare November 7, 2023 15:05
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch from df0a573 to dbad54a Compare November 8, 2023 11:46
sn_node/tests/nodes_rewards.rs Fixed Show fixed Hide fixed
sn_node/tests/nodes_rewards.rs Fixed Show fixed Hide fixed
@joshuef
Copy link
Contributor

joshuef commented Nov 13, 2023

Marking as DoNotMerge so we can assess this...

Does it lower mem usage any? CPU? Would be good to know concretely what we get from this.

@maqi maqi force-pushed the publish_to_non_subscribed_topic branch 3 times, most recently from 7aa6193 to f2a6ccc Compare November 14, 2023 10:26
sn_node/tests/nodes_rewards.rs Outdated Show resolved Hide resolved
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch from f2a6ccc to 9257008 Compare November 16, 2023 09:50
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch from 9257008 to 973335e Compare November 28, 2023 14:25
@reviewpad reviewpad bot added Small Pull request is small and removed Medium Medium sized PR labels Nov 28, 2023
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch 2 times, most recently from 067017f to 9ae3b77 Compare November 29, 2023 11:51
@maqi maqi removed the DoNotMerge label Nov 29, 2023
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch from 70dca4a to b38c3c6 Compare November 29, 2023 13:44
@maqi maqi force-pushed the publish_to_non_subscribed_topic branch from b38c3c6 to 4994dee Compare November 29, 2023 15:08
@maqi maqi dismissed bochaco’s stale review November 29, 2023 16:05

request resolved

@joshuef joshuef added this pull request to the merge queue Nov 29, 2023
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 29, 2023
@maqi maqi added this pull request to the merge queue Nov 29, 2023
Merged via the queue into maidsafe:main with commit 00601bc Nov 29, 2023
28 checks passed
@maqi maqi deleted the publish_to_non_subscribed_topic branch November 29, 2023 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Small Pull request is small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants