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: fix encoder/decoder creation #1742

Merged
merged 3 commits into from
Nov 29, 2023
Merged

Conversation

danisharora099
Copy link
Collaborator

Problem

With #1697, setting pubsub topic based on shardInfo was introduced.
a bug was spotted during property check where if the index is 0, it falls back to the DefaultPubsubTopic.

Solution

The PR removes the property check, and just checks the definition on pubsubTopicShardInfo

Another solution could be to add a specific unedefined check like: pubsubTopicShardInfo !== undefined but there are very little benefits of additional property checking on a type-safe system. (ref: #1697 (comment))

@danisharora099 danisharora099 requested a review from a team as a code owner November 29, 2023 09:33
@danisharora099 danisharora099 changed the title chore: fix encoder creation chore: fix encoder/decoder creation Nov 29, 2023
Copy link

github-actions bot commented Nov 29, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
Waku core 78.36 KB (-0.01% 🔽) 1.6 s (-0.01% 🔽) 223 ms (+37.18% 🔺) 1.8 s
Waku Simple Light Node 240.88 KB (-0.01% 🔽) 4.9 s (-0.01% 🔽) 333 ms (+40.82% 🔺) 5.2 s
ECIES encryption 72.84 KB (-0.01% 🔽) 1.5 s (-0.01% 🔽) 226 ms (+86.38% 🔺) 1.7 s
Symmetric encryption 72.82 KB (-0.01% 🔽) 1.5 s (-0.01% 🔽) 298 ms (+96.64% 🔺) 1.8 s
DNS discovery 120.85 KB (0%) 2.5 s (0%) 328 ms (+116.73% 🔺) 2.8 s
Privacy preserving protocols 125.75 KB (0%) 2.6 s (0%) 218 ms (+31.31% 🔺) 2.8 s
Light protocols 75.93 KB (-0.01% 🔽) 1.6 s (-0.01% 🔽) 277 ms (+131.16% 🔺) 1.8 s
History retrieval protocols 74.85 KB (-0.01% 🔽) 1.5 s (-0.01% 🔽) 164 ms (+136.26% 🔺) 1.7 s
Deterministic Message Hashing 5.65 KB (0%) 113 ms (0%) 75 ms (+320.97% 🔺) 188 ms

@weboko
Copy link
Collaborator

weboko commented Nov 29, 2023

Isn't it is fixed in #1740?

@danisharora099
Copy link
Collaborator Author

Isn't it is fixed in #1740?

#1740 is dependent on this PR. removed the commit fixing it there and this should be merged as a separate PR

@danisharora099 danisharora099 merged commit 7ce642c into master Nov 29, 2023
9 of 11 checks passed
@danisharora099 danisharora099 deleted the fix/encoder-creation branch November 29, 2023 12:00
danisharora099 added a commit that referenced this pull request Nov 29, 2023
* fix encoder creation

* bump nwaku to v0.22.0 (#1741)
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.

2 participants