-
Notifications
You must be signed in to change notification settings - Fork 543
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
try to fix the staged_ledger test #14058
try to fix the staged_ledger test #14058
Conversation
!ci-build-me |
!ci-build-me |
!ci-build-me |
!ci-build-me |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Requested some changes for better readability but otherwise looks good
@@ -37,7 +37,22 @@ let zkapp_command_with_ledger ?num_keypairs ?max_account_updates | |||
Option.value num_keypairs | |||
~default:((max_account_updates * 2) + (max_token_updates * 3) + 2) | |||
in | |||
let keypairs = List.init num_keypairs ~f:(fun _ -> Keypair.create ()) in | |||
let existing_keypairs = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is not used elsewhere, could you move this under the keypairs
block of code?
Array.map l ~f:(fun (kp, _balance, _nonce, _timing) -> kp) | ||
|> Keypair.Set.of_array ) | ||
in | ||
let keypairs = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename this to new_keypairs
?
…tocol/mina into fix/staged-ledger-test-zkapp-gen
…ed-ledger-test-zkapp-gen
!ci-build-me |
!ci-build-me |
…tocol/mina into fix/staged-ledger-test-zkapp-gen
!ci-build-me |
!ci-build-me |
!ci-build-me |
!ci-build-me |
|> Keypair.Set.of_array ) | ||
in | ||
List.init num_keypairs ~f:(fun _ -> | ||
let keypair = ref (Keypair.create ()) in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quickcheck generators are supposed to be deterministic, but Keypair.create
will be different each time. Can you fix this so that the generator give the same output for the same input seed?
…tocol/mina into fix/staged-ledger-test-zkapp-gen
…ed-ledger-test-zkapp-gen
!ci-build-me |
!approved-for-mainnet |
Explain your changes:
This PR tries to fix a staged_ledger unit test bug. The generation of zkApps and user_commands sometimes could crash in their pick of keypairs. This PR fixes that.
Explain how you tested your changes:
*
Checklist: