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

(Contracts) Consolidate events 4: fix facades (tests) #178

Open
1 task
memarin opened this issue Jan 16, 2025 · 0 comments
Open
1 task

(Contracts) Consolidate events 4: fix facades (tests) #178

memarin opened this issue Jan 16, 2025 · 0 comments
Assignees

Comments

@memarin
Copy link

memarin commented Jan 16, 2025

Sub Task 4:

🏗️ This will break a lot of the testing suite, to fix this, we only need to adjust the facades (I think only [the option round facade](

let bid: Bid = self.option_round_dispatcher.place_bid(amount, price);
) will need to be adjusted). The facade is setup to wrap the contract, and is what most tests interact with instead of the contract directly. Adjusting the facade should fix most broken tests, but there may be some edge cases that require the test directly to be adjusted. For example, in the link above for the option round facade, instead of calling self.option_round_dispatcher.place.bid(), we should adjust this to calling self.get_vault_facade().place_bid() . This will be required for all of the above functions’ facade functions, as well as the xxx_expect_error() versions of the functions.

  • Fix facades so that all tests are back to passing

Disclaimer

🚨 Way earlier we introduced ‘sanity_checks’ which were designed to verify that the returned result of each function matches what is stored in the contract (every time it is called). For example, [the place bid sanity check](

fn place_bid(ref self: OptionRoundFacade, bid: Bid) -> Bid {
) asserts that the bid_id returned from placing a bid matches the expected bid_id from the hashing. Off the top of my head I do not think these need adjustment, but keep them in mind if some tests remain broken.

  • The original idea was that the sanity checks could catch errors that unit tests might not be designed to catch, but currently unsure if this is still necessary.
  • We can consider removing sanity checks and adding unit tests instead to remove bloat, but is not necessary for this task.
@dhruv035 dhruv035 transferred this issue from OilerNetwork/pitchlake-ui-new Jan 20, 2025
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

2 participants