Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix multi-protocol commitments merge-reveal and conceal procedures #129

Merged
merged 24 commits into from
Jul 10, 2023

Conversation

dr-orlovsky
Copy link
Member

@dr-orlovsky dr-orlovsky commented Jul 10, 2023

This is downstream compilation non-breaking part of #127

NB: While it doesn't break compilation downstream, please keep in mind that it still breaks consensus rules, changes APIs (which were not yet used downstream) and invalidates existing stashes and inventories (i.e. stocks).

@dr-orlovsky dr-orlovsky added bug Something isn't working test Test implementation or failures *security* Issues affecting safety/security (include undefined behaviours) *compatibility* Issues affecting compatibility and interoperability *consensus* Issues affecting distributed concensus labels Jul 10, 2023
@dr-orlovsky dr-orlovsky added this to the v0.10.x milestone Jul 10, 2023
@dr-orlovsky dr-orlovsky requested a review from zoedberg July 10, 2023 09:27
Copy link
Member

@zoedberg zoedberg left a comment

Choose a reason for hiding this comment

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

I've run the failing test 100 times and it always succeeded, so I think we can merge this. Thank you @dr-orlovsky :)

In these days we'll write some more tests to increase chances that everything works good and no more breaking changes will be needed.

@dr-orlovsky
Copy link
Member Author

The trickiest part were the problems happening when 3+ assets are involved. So I will also be working on increasing test coverage to scenarious above 100 assets. However, right now we can't have 100+ assets in a stable way b/c of #128 (we can't guarantee that all 100+ assets have a unique modulo divider within a scope of powers of two from 1 to 16).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working *compatibility* Issues affecting compatibility and interoperability *consensus* Issues affecting distributed concensus *security* Issues affecting safety/security (include undefined behaviours) test Test implementation or failures
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants