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

offchainState settle() throws error when remaining action exceed maxActionsPerProof #1887

Open
kadirchan opened this issue Oct 26, 2024 · 3 comments

Comments

@kadirchan
Copy link
Contributor

kadirchan commented Oct 26, 2024

When I'm running stress test for my DRM Contract, offchainState gives me error:

actions for DRM1 8
Error when proving DRM.settle()
.../o1js/dist/node/bindings/compiled/_node_bindings/o1js_node.bc.cjs:6761
         throw err;
         ^

[Error: Constraint unsatisfied (unreduced):
field_inverse
Checked.inv
assert_non_zero: File "src/mina/src/lib/snarky/src/base/utils.ml", line 174, characters 2-106
any: File "src/mina/src/lib/snarky/src/base/utils.ml", line 392, characters 6-97
File "src/mina/src/lib/pickles/wrap_main.ml", line 355, characters 23-30
File "src/mina/src/lib/pickles/wrap_main.ml", line 167, characters 17-24

Constraint:
((basic(R1CS(Add(Var 13457)(Add(Constant 1)(Scale 28948022309329048855892746252171976963363056481941647379679742748393362948096(Var 109))))(Var 13459)(Constant 1)))(annotation(field_inverse)))
Data:
R1CS 0 0 1]

and its disappear when I increase my maxActionsPerProof to 10 from 5. I think current implementation of createSettlementProof() does not paying attention to maxActionsPerProof constraints when current unsettled action size exceeds.

@kadirchan
Copy link
Contributor Author

Test file: https://github.com/DRM-Mina/drm-mina-web-client/blob/7a8f73da3724f392843a5b1b5522e13cd6f325ba/contracts/src/test/BundleTest.ts#L235
To receive error reconfigure maxActionsPerProof to 5 again in DRM.ts and run this test.

@mitschabaude
Copy link
Collaborator

I just wanted to write that we have a unit test which checks the case where pending actions exceed the actionsPerProof, but @45930 it seems that you removed that test in #1834, and replaced it with tests that don't check that important case? Seems essential to have such a test

@Trivo25
Copy link
Member

Trivo25 commented Oct 28, 2024

For visibility @boray @Shigoto-dev19 @hattyhattington17

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

No branches or pull requests

3 participants