-
Notifications
You must be signed in to change notification settings - Fork 6
test(multi-collateral): add transfer utils #178
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
test(multi-collateral): add transfer utils #178
Conversation
f71a46b to
d075e76
Compare
d075e76 to
e130279
Compare
9824ef8 to
e2d572a
Compare
omri-ba-starkware
left a comment
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.
@omri-ba-starkware reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ishay-starkware and @RoeeGross).
workspace/apps/perpetuals/contracts/src/tests/flow_tests/perps_tests_facade.cairo line 1151 at r1 (raw file):
asset_id: self.collateral_id, :amount, caller: sender,
So in our tests the sender is always the one who calls the request and never the receiver?
Code quote:
caller: sendere2d572a to
21a7b1a
Compare
MohammadNassar1
left a comment
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.
@MohammadNassar1 made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ishay-starkware and @RoeeGross).
workspace/apps/perpetuals/contracts/src/tests/flow_tests/perps_tests_facade.cairo line 1151 at r1 (raw file):
Previously, omri-ba-starkware wrote…
So in our tests the sender is always the one who calls the request and never the receiver?
Good point. I initialy copied the withdraw/deposit behavior, but in the transfer, the user only calls the function, so testing different callers isn’t necessary.
I renamed the internal function and clarified that it’s not meant to be called by the test.
omri-ba-starkware
left a comment
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.
@omri-ba-starkware reviewed 1 file and all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @ishay-starkware and @RoeeGross).
Note
Introduces multi-collateral transfer support in flow tests and validates balances for non-base assets.
transfer_spot_requesttoinfra.cairoandperps_tests_facade.cairo; refactor into_transfer_requestthat accepts an explicitasset_idtransferpath to branch on base collateral vs spot (get_position_collateral_balancevsget_position_asset_balance) and validate withvalidate_asset_balancefor non-base assets; adjust events to use providedasset_idtest_spot_collateral_deposit_transfer_withdrawcovering STRK spot asset deposit, partial transfer, and withdrawals with final balance assertionsWritten by Cursor Bugbot for commit 21a7b1a. This will update automatically on new commits. Configure here.
This change is