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

Implementing StrictBank functions and tests #426

Merged
merged 12 commits into from
Oct 6, 2023

Conversation

VictorONN
Copy link
Contributor

Related to #272

Implemented functions to the best of my understanding of the GMX contracts. The implementation I think I did well the tests are the issue. The functions are:
record_transfer_in
sync_token_balance
record_transfer_in_internal
after_transfer_out_infernal

PASSING TESTS
The ones which are passing and am comfy with the logic:
fn test_initialize()
fn given_normal_conditions_when_transfer_out_then_works()
fn given_caller_has_no_controller_role_when_transfer_out_then_fails()
fn given_caller_has_no_controller_role_when_record_transfer_in_then_fails()
fn given_caller_has_no_controller_role_when_sync_token_balance_then_fails()

fn given_normal_conditions_when_sync_token_balance_passes() - this passes but am unsure if I have to play around with token balance first to at least test it or I just call it directly.

FAILING TESTS

fn given_receiver_is_contract_when_transfer_out_then_fails() - Error: (u256_sub Overflow) I think am having the logic correct, am minting to Bank contract address, and passing the bank contract address to transfer out function to make it panic with the assert reason.

fn given_normal_conditions_when_record_transfer_in_works() 'ERC20: transfer from 0' error. It seems it is not minting to caller address successfully. That is why for this test I have I tried to mint to the strict bank address, and transfer out, then transfer in again to register the change. Still getting 'ERC20: transfer from 0'

@tevrat-aksoy
Copy link
Contributor

hello, you implemented _afterTransferOut but didnt used anywhere. Since it is override function I think you need to use it after you called IBank::transfer_out() function.
https://github.com/VictorONN/satoru/blob/6711409640ff7e19fdfe26b7856370734cbc70ad/src/bank/strict_bank.cairo#L129C13-L129C32

@sparqet sparqet linked an issue Sep 24, 2023 that may be closed by this pull request
@sparqet
Copy link
Collaborator

sparqet commented Oct 4, 2023

@VictorONN Can you fix your coding style pls ?

@sparqet
Copy link
Collaborator

sparqet commented Oct 5, 2023

@VictorONN do you have any news ? If I don't receive a response within 24 hours, I will have to proceed with redoing the issue

@VictorONN
Copy link
Contributor Author

Hello, fixed style and fixed the tests too.

@sparqet sparqet merged commit 3367de4 into keep-starknet-strange:main Oct 6, 2023
3 checks passed
StarkFishinator pushed a commit to StarkFishinator/satoru that referenced this pull request Oct 9, 2023
* strict bank start

* strict bank contract and tests

* All strict bank tests running

* formatted

---------

Co-authored-by: Michel <105498726+Sk8erboi84@users.noreply.github.com>
Co-authored-by: sparqet <37338401+sparqet@users.noreply.github.com>
Co-authored-by: zarboq <37303126+zarboq@users.noreply.github.com>
zarboq added a commit that referenced this pull request Nov 5, 2023
* exp function, no tested (failed setting up runner)

* not working :(

* error comes from 1e18?

* exp2 working git add .

* exp is working !

* fmt

* Feat:  Implement the function in the market_utils library #3 (#463)

* 10 functions done

* almost finished, debug next

* debug time

* debuging

* pushing recent changes/ still bug because missing functions

* debuging finished

* adding comments on functions

* almost clean

* Emit bug

* programm compile 🎉

* resolving last test

* All test passed

* resolve request

* 1 test failed because of max swap path lenght exceed test

* resolving failed test

* resolve

* solving

* compilation resolved

---------

Co-authored-by: Michel <105498726+Sk8erboi84@users.noreply.github.com>

* test: Tests for deposit_vault (#496)

* test(deposit_vault_contract): Can start doing some unit tests from that

* test(deposit_vault_contract): Better format

* test(deposit_vault_contract): All test passes! 🥳

* test(deposit_vault_contract): Added unit test for transfer_out not enough balance

---------

Co-authored-by: akhercha <akherchache@pm.me>
Co-authored-by: sparqet <37338401+sparqet@users.noreply.github.com>

* test: Improve tests for referral_storage contract (#499)

* test(referral_storage_contract): Refacto test architecture before adding tests

* test(referral_storage_contract): Quick refacto of base tests

* test(referral_storage_contract): Added more tests

* test(referral_storage_contract): Finished tests for referral_storage contract

* test(referral_storage_contract): Removed custom teardown

* test(referral_storage_contract): Unused imports

* test(referral_storage_contract): Updated top comment

* test(referral_storage_contract): from review; added init check in Governable

* test(referral_storage_contract): Removed useless line

* test(referral_storage_contract): Removed useless line x2 💀

---------

Co-authored-by: akhercha <akherchache@pm.me>

* test: Improve tests of referral_utils library. #483 (#498)

* update referalla-util tests

* allow changing refferal

* remove role_store

---------

Co-authored-by: zarboq <37303126+zarboq@users.noreply.github.com>

* Implementing StrictBank functions and tests (#426)

* strict bank start

* strict bank contract and tests

* All strict bank tests running

* formatted

---------

Co-authored-by: Michel <105498726+Sk8erboi84@users.noreply.github.com>
Co-authored-by: sparqet <37338401+sparqet@users.noreply.github.com>
Co-authored-by: zarboq <37303126+zarboq@users.noreply.github.com>

* ✨ execute_deposit_utils functions (#449)

* ✨ execute_deposit_utils fn

* fix: clone fees and rm extra line

* fix: prevent BadMergeBaseMismatch by adding else to if-

* fix: fmt issue

* Feat: Adding a Contirbutor profil (#501)

* 10 functions done

* almost finished, debug next

* debug time

* debuging

* pushing recent changes/ still bug because missing functions

* debuging finished

* adding comments on functions

* almost clean

* Emit bug

* programm compile 🎉

* resolving last test

* All test passed

* resolve request

* 1 test failed because of max swap path lenght exceed test

* resolving failed test

* resolve

* solving

* compilation resolved

* Added as a Contributor

* Adding profil on ReadMe

---------

Co-authored-by: Michel <105498726+Sk8erboi84@users.noreply.github.com>

* test: Added tests for record_transfer_in (#502)

* test(record_transfer_in_function): Added unit tests for record_transfer_in

* test(record_transfer_in_function): Added unit tests

* test(record_transfer_in_function): Better error message

* test(record_transfer_in_function): Mock + Overflow prevented

* test(record_transfer_in_function): Removed useless import

* test(record_transfer_in_function): record_transfer_in panic on sub overflow

* test(record_transfer_in_function): Quick test refacto

* test(record_transfer_in_function): Unused variables in tests

---------

Co-authored-by: akhercha <akherchache@pm.me>
Co-authored-by: zarboq <37303126+zarboq@users.noreply.github.com>

* Improve tests of governable contract (#503)

* add test to gov

* fmt fix

* add new contributors (#505)

* change panic error

* change quote

* log2 function is working

* fmt

* fmt

* pow working

* fmt

* convert type

* fix

* lock

* fmt + lock

* works

---------

Co-authored-by: Tbelleng <117627242+Tbelleng@users.noreply.github.com>
Co-authored-by: Michel <105498726+Sk8erboi84@users.noreply.github.com>
Co-authored-by: akhercha <adelkherchache@gmail.com>
Co-authored-by: akhercha <akherchache@pm.me>
Co-authored-by: sparqet <37338401+sparqet@users.noreply.github.com>
Co-authored-by: tevrat aksoy <tevrataksoy@gmail.com>
Co-authored-by: zarboq <37303126+zarboq@users.noreply.github.com>
Co-authored-by: VictorONN <73134512+VictorONN@users.noreply.github.com>
Co-authored-by: kasteph <ayo@kasteph.com>
Co-authored-by: Axel Izsak <98711930+axelizsak@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

Feat: Implement the functions in the StrictBank contract.
5 participants