Skip to content

Update rsmt2d in IPLD plugin#290

Merged
Wondertan merged 2 commits intomasterfrom
hlib/update-plugin-rsmt2d
Apr 20, 2021
Merged

Update rsmt2d in IPLD plugin#290
Wondertan merged 2 commits intomasterfrom
hlib/update-plugin-rsmt2d

Conversation

@Wondertan
Copy link
Member

Also, previously plugin used rsmt2d v0.1.1-0.20210327010029-ef1d6c54461e which does not have NewRSGF8Code func used by latest plugin tests, making the plugin not able to compile, thus not able to run tests. However, the test on CI looks good, this brings the thought that our CI does not run tests in IPLD plugin. Am I missing something or our CI really ignores the IPLD plugin?

@Wondertan Wondertan requested review from evan-forbes and liamsi April 20, 2021 13:15
@Wondertan Wondertan self-assigned this Apr 20, 2021
@Wondertan Wondertan requested a review from tac0turtle as a code owner April 20, 2021 13:15
@evan-forbes
Copy link
Member

Dang, I think you're right, good find!! The CI isn't calling the tests for the plugin. Also, this was my fault for not calling those tests after using the new rsmt2d. 😅

I think the best way to make sure the tests get called is to add them to the testing Makefile in test/Makefile. Do you mind adding that in this PR? Or spinning up a new issue so we can address that there? thanks!

github.com/lazyledger/nmt v0.3.1
// rsmt2d is only used in tests:
github.com/lazyledger/rsmt2d v0.1.1-0.20210327010029-ef1d6c54461e
github.com/lazyledger/rsmt2d v0.2.0
Copy link
Member

Choose a reason for hiding this comment

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

👍🏼

Copy link
Member

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

Thanks! Good find! This looks good to me.

Regarding the tests: I think, we should remove the sub-module (in a separate PR), then, I think the tests under p2p/ipld/nodes will also be ran by CI without any further work.

The rationale behind this structure (the plugin being its own go-module) was that we wanted to test the plgin in the ipld-experiments without depending on the full lazyledger-core module. Looking at this now, it seems like this was a somewhat misguided decision as for the actual experiments it should not matter. Later we can think about putting the plugin into it's own repo instead.

@Wondertan Wondertan merged commit e31cc98 into master Apr 20, 2021
@Wondertan Wondertan deleted the hlib/update-plugin-rsmt2d branch April 20, 2021 14:23
cmwaters pushed a commit that referenced this pull request Mar 13, 2023
* Updated method for running 200 node test

* fix & extend

* one more...

* update hashes

* Adapting text of latency extraction method

* Script adjustments

* Small precision

* Update docs/qa/method.md

Thanks!

Co-authored-by: Lasaro <lasaro@informal.systems>

* Update docs/qa/method.md

Co-authored-by: Lasaro <lasaro@informal.systems>

* Update docs/qa/method.md

Co-authored-by: Lasaro <lasaro@informal.systems>

---------

Co-authored-by: Lasaro <lasaro@informal.systems>
(cherry picked from commit d82bc77)

Co-authored-by: Sergio Mena <sergio@informal.systems>
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