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

PR #330 - enable test verkle ipa #389

Merged
merged 6 commits into from
Jun 7, 2024
Merged

Conversation

mratsim
Copy link
Owner

@mratsim mratsim commented Jun 7, 2024

This is #330, rebased to latest master and for investigation of CI failure on Windows

@mratsim mratsim changed the title PR #33) - enable test verkle ipa PR #330 - enable test verkle ipa Jun 7, 2024
@mratsim
Copy link
Owner Author

mratsim commented Jun 7, 2024

Bisecting, the regression mentioned in #330 (comment) is due to #371

@mratsim mratsim force-pushed the pr330_enable_test_verkle_ipa branch from 44bd3a3 to dc2084f Compare June 7, 2024 13:36
@mratsim
Copy link
Owner Author

mratsim commented Jun 7, 2024

Confirmed that Windows failure without any error is due to a stack overflow due to array[VerkleDomain, array[VerkleDomain, Fr[Banderwagon]] being too large for Windows stack.

The temporary workaround is to use a ref array, see bc74902.

Ultimately those should be replaced by using Constantine allocator functions, to avoid Nim allocator, see: #367

Strangely, if changing to a ref alloc in the test file, the Multiproof creation and verification test fails. Hopefully switching 1 out of 2 to heap space leaves enough stack space for Windows test.

@mratsim mratsim marked this pull request as ready for review June 7, 2024 13:41
@mratsim mratsim merged commit 4e1ea61 into master Jun 7, 2024
12 checks passed
@mratsim mratsim deleted the pr330_enable_test_verkle_ipa branch June 7, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants