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

chore!: update proto definitions #1196

Merged
merged 9 commits into from
Feb 27, 2023
Merged

Conversation

fryorcraken
Copy link
Collaborator

@fryorcraken fryorcraken commented Feb 24, 2023

Use protobuf definitions from https://github.com/vacp2p/waku

Ref: #335

@fryorcraken fryorcraken force-pushed the chore/update-proto-definition branch from 2b66cba to affd3d6 Compare February 24, 2023 05:01
@github-actions
Copy link

github-actions bot commented Feb 24, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 114.41 KB (+0.09% 🔺) 2.3 s (+0.09% 🔺) 3.9 s (+61.27% 🔺) 6.1 s
Waku default setup 366.93 KB (+0.03% 🔺) 7.4 s (+0.03% 🔺) 7.5 s (+43.94% 🔺) 14.9 s
ECIES encryption 27.89 KB (+0.04% 🔺) 558 ms (+0.04% 🔺) 1.8 s (+23.44% 🔺) 2.4 s
Symmetric encryption 27.89 KB (+0.05% 🔺) 558 ms (+0.05% 🔺) 2.6 s (+34.67% 🔺) 3.1 s
DNS discovery 108.79 KB (0%) 2.2 s (0%) 3.6 s (+10.02% 🔺) 5.8 s
Privacy preserving protocols 113.88 KB (+0.09% 🔺) 2.3 s (+0.09% 🔺) 3.3 s (-3.63% 🔽) 5.6 s
Light protocols 115.75 KB (+0.06% 🔺) 2.4 s (+0.06% 🔺) 2.5 s (-0.72% 🔽) 4.8 s
History retrieval protocols 115.73 KB (+0.05% 🔺) 2.4 s (+0.05% 🔺) 2.8 s (-2.64% 🔽) 5.1 s

@fryorcraken fryorcraken changed the title chore: update proto definitions chore!: update proto definitions Feb 24, 2023
@fryorcraken fryorcraken force-pushed the chore/update-proto-definition branch from abe44a7 to 2710889 Compare February 24, 2023 12:39
@fryorcraken fryorcraken marked this pull request as ready for review February 24, 2023 13:01
@fryorcraken
Copy link
Collaborator Author

Cc @LNSD @richard-ramos

@@ -408,7 +404,7 @@ export async function createCursor(
digest,
pubsubTopic,
senderTime: messageTime,
receivedTime: messageTime,
receiverTime: messageTime,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I am surprised that using the waku message timestamp here works. Cc @LNSD @danisharora099

@richard-ramos
Copy link
Member

@fryorcraken. would it make sense to instead of including the .proto files in the project, to use a git submodule pointing to vacp2p/waku?

@@ -8,16 +8,16 @@ export type ContentFilter = {
/**
* FilterRPC represents a message conforming to the Waku Filter protocol
*/
export class FilterRPC {
public constructor(public proto: proto.FilterRPC) {}
export class FilterRpc {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you change capitalisation here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Capitalization was changed on proto file to proper PascalCase, I just match it as PascalCase is what we use across the codebase.

@fryorcraken
Copy link
Collaborator Author

@fryorcraken. would it make sense to instead of including the .proto files in the project, to use a git submodule pointing to vacp2p/waku?

Maybe. There is an issue with the peer exchange definition (I need to create an issue) hence we cannot use the repo as is just yet.

The other question is whether protons can support proper protobuf module path, I don't know.

Let's review once peer exchange is done.

@fryorcraken
Copy link
Collaborator Author

Maybe. There is an issue with the peer exchange definition (I need to create an issue)

The issue: waku-org/waku-proto#14

@fryorcraken fryorcraken force-pushed the chore/update-proto-definition branch from 357f9ca to 81d6638 Compare February 27, 2023 03:01
@fryorcraken fryorcraken merged commit 7a3c9a8 into master Feb 27, 2023
@fryorcraken fryorcraken deleted the chore/update-proto-definition branch February 27, 2023 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants