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

added broadcast api call and test #494

Merged
merged 4 commits into from Feb 4, 2019

Conversation

Projects
None yet
2 participants
@y0sher
Copy link
Collaborator

y0sher commented Jan 29, 2019

No description provided.

@y0sher y0sher force-pushed the automation_rpc_broadcast branch from 222de01 to 2559397 Jan 29, 2019

@y0sher y0sher requested review from antonlerner and zalmen Jan 29, 2019

@y0sher y0sher force-pushed the automation_rpc_broadcast branch from 2559397 to 10c9d76 Feb 3, 2019

@zalmen zalmen self-assigned this Feb 3, 2019

@zalmen
Copy link
Member

zalmen left a comment

Currently only messages that are valid according to the specified protocol will be propagated, which isn't what this call is meant to do. You should add a dedicated protocol that will process and approve any message and propagate it

@@ -65,6 +65,16 @@ func (s SpaceMeshGrpcService) SubmitTransaction(ctx context.Context, in *pb.Sign
return &pb.SimpleMessage{Value:"ok"}, nil
}

// P2P API

func (s SpaceMeshGrpcService) Broadcast(ctx context.Context, in *pb.BroadcastMessage) (*pb.SimpleMessage, error) {

This comment has been minimized.

@zalmen

zalmen Feb 3, 2019

Member

I know it's not part of this PR - but it should be Spacemesh and not SpaceMesh

string Protocol = 1;
bytes Data = 2;
}

service SpaceMeshService {

This comment has been minimized.

@zalmen

zalmen Feb 3, 2019

Member

Same comment - should be Spacemesh

@y0sher y0sher force-pushed the automation_rpc_broadcast branch from 686e125 to 87e45a8 Feb 4, 2019

@y0sher

This comment has been minimized.

Copy link
Collaborator Author

y0sher commented Feb 4, 2019

addressed your comments @zalmen

@zalmen

zalmen approved these changes Feb 4, 2019

app/main.go Outdated
@@ -242,6 +242,10 @@ func (app *SpacemeshApp) startSpacemesh(cmd *cobra.Command, args []string) {
// start p2p services
log.Info("Initializing P2P services")
swarm, err := p2p.New(Ctx, app.Config.P2P)

// Gossip messages are validated before propagating. broadcast test protocol isn't
api.ApproveAPIGossipMessages(Ctx, swarm) // todo: maybe only enable in tests?

This comment has been minimized.

@zalmen

zalmen Feb 4, 2019

Member

I agree with the TODO, we wouldn't want to allow anyone to spam the network. It should be configurable with a default to false

@y0sher y0sher force-pushed the automation_rpc_broadcast branch from 87e45a8 to 4d06442 Feb 4, 2019

@y0sher y0sher force-pushed the automation_rpc_broadcast branch from 4d06442 to f8ff90a Feb 4, 2019

@y0sher y0sher merged commit 470b851 into develop Feb 4, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment