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

test: countable (switchable, ownable) components #205

Merged
merged 7 commits into from
Jul 1, 2024

Conversation

NueloSE
Copy link
Contributor

@NueloSE NueloSE commented May 31, 2024

Issue(s): #70

Description

Please provide a brief description of the changes made in this pull request and how they address the related issue.

Checklist

  • CI Verifier: Run ./scripts/cairo_programs_verifier.sh successfully
  • Contract Tests: Added tests to cover the changes

@NueloSE NueloSE marked this pull request as ready for review June 1, 2024 12:34
@NueloSE
Copy link
Contributor Author

NueloSE commented Jun 1, 2024

@julio4 ready for review

@NueloSE
Copy link
Contributor Author

NueloSE commented Jun 4, 2024

@julio4 ready for review

Copy link
Contributor

@julio4 julio4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!
Could you:

  • Add test cases to verify that events are emitted
  • Wrap contracts/components inside anchor comments and edit the corresponding markdown files in src to use rustdoc_include ...:anchor to hide by default tests in the book

@julio4
Copy link
Contributor

julio4 commented Jun 13, 2024

@NueloSE Do you need any help to finish this?

@NueloSE
Copy link
Contributor Author

NueloSE commented Jun 13, 2024

@julio4
Yes I need help I haven't been able to figure how to add test to test emitted events for switchable and ownable

@julio4
Copy link
Contributor

julio4 commented Jun 15, 2024

@julio4 Yes I need help I haven't been able to figure how to add test to test emitted events for switchable and ownable

Read the tests of this section. (Be sure to toggle hidden lines)

@NueloSE
Copy link
Contributor Author

NueloSE commented Jun 19, 2024

Hi @julio4

  • I have added the ANCHOR tags
  • Implemented the test for emitted events in the reference sample but the test keeps failing for the emitted events.
    A screenshot has been attached of the error
    Screenshot from 2024-06-19 10-43-37

listings/applications/components/src/countable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/countable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/countable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/countable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/countable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
@0xNeshi
Copy link
Contributor

0xNeshi commented Jun 21, 2024

Hi @julio4

  • I have added the ANCHOR tags
  • Implemented the test for emitted events in the reference sample but the test keeps failing for the emitted events.
    A screenshot has been attached of the error
    Screenshot from 2024-06-19 10-43-37

@julio4 I tested this locally and I experienced the same issues. It's like events are not being emitted from components.

@NueloSE could you try updating your tests to use Foundry (see Testing Smart Contracts)? Maybe these are some issues that they solved.

I tested this out with Foundry too, and found that there's a bug when spying on events emitted from components. Created a ticket for this to be investigated further, see starkware-libs/cairo#5861.

@julio4
Copy link
Contributor

julio4 commented Jun 24, 2024

I tested this out with Foundry too, and found that there's a bug when spying on events emitted from components. Created a ticket for this to be investigated further, see starkware-libs/cairo#5861.

Good find, we can just comment the event spying for now.

Copy link
Contributor

@0xNeshi 0xNeshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes required to make the original test suite pass (without using #[flat])

listings/applications/components/src/ownable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/ownable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/ownable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/ownable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/ownable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
listings/applications/components/src/switchable.cairo Outdated Show resolved Hide resolved
@NueloSE
Copy link
Contributor Author

NueloSE commented Jun 27, 2024

Ready for Review

Copy link
Contributor

@0xNeshi 0xNeshi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful!

Let's just wait until @julio4 gives it one final look, and that's it 🚀

@julio4 julio4 changed the title test: implement test for countable component test: countable, switchable, ownable components Jul 1, 2024
@julio4 julio4 changed the title test: countable, switchable, ownable components test: countable (switchable, ownable) components Jul 1, 2024
Copy link
Contributor

@julio4 julio4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding test for countable and updating a little bit the switchable and ownable tests.
I cleaned up some files, feel free to check and I'll merge if everything is ok!

@NueloSE
Copy link
Contributor Author

NueloSE commented Jul 1, 2024

lgtm

@julio4 julio4 merged commit 6f4d055 into NethermindEth:main Jul 1, 2024
1 check passed
julio4 added a commit to LamsyA/StarknetByExample that referenced this pull request Oct 24, 2024
* test: implement test for countable component

* test: implement test for switchable component

* test: implement test for ownable component

* chore: implement test for emitted events and add anchor tags

* feat: apply requested changes

* feat: apply requested changes

* feat: revisions

---------

Co-authored-by: julio4 <[email protected]>
julio4 added a commit that referenced this pull request Oct 24, 2024
* commitment scheme implementation

* scarb.lock

* modified the reveal function to view

* refactored

* refactored

* offchain commitment and onchain verification done

* added erc20 unit test

* added test against custom errors

* formated

* made requested changes and moved test to contract module

* fixed zero address

* chore: Remove unused test files

* Library calls (#194)

* final push

* updated the Summary.md

* fix: fmt and minor edit

---------

Co-authored-by: julio4 <[email protected]>

* Chapter 0 > Basics: wording + grammar fixes (#207)

* sierra->Sierra + plural/singular form fixes

* ch00 > Missing attr. elems, wrong attr. names

* missing dot in messages + update how legacymap modulo format

* ch00 > errors > add indent to comment in complex section

* ch00 > events > wording, missing code quotes

* ch00 > syscalls > wording

* ch00 > bytearray > wording

* ch00 > stor.custom types > wording

* ch00 > cust.types in entrypoints > wording

* ch00 > documentation > wording

* Revert comment format changes

* Simplify panic_with_felt252 related comment in errors.md

* Fixes for ch00

* Wording in cheatsheet

* Comment update in type_casting

---------

Co-authored-by: Nenad <[email protected]>

* component dependencies test (#198)

* test: component dependencies

* undo changes

* test: countable_component

* test: countable_internal_dep_switch

* fix: apply requested changes

* fix: small fmt fix

---------

Co-authored-by: julio4 <[email protected]>

* feat(staking): Add staking contract example (#202)

* feat(staking): first draft with contract and tests

Missing events and some tests

* feat(staking): Add events and events-related tests

* feat(staking): Add a more complex test for rewards set up

* feat(staking): Add md file

* feat(staking): Apply changes according to PR review

* feat(enums): Enums in contract (#212)

* feat(enums): Enums in contract

* fix: small fmt

---------

Co-authored-by: julio4 <[email protected]>

* Application chapter: grammar, wording, typos, `!addr.is_zero` -> `addr.is_non_zero` (#213)

* upgradeable > use is_non_zero

* simple_vault > grammar

* not is_zero -> is_non_zero

* update sbe links

* staking > wording

* amm > fix brackets + grammar

---------

Co-authored-by: Nenad <[email protected]>

* add time locked transactions example (#201)

* add time locked transactions example

* install snforge in gh-action

* fix: refactor timelock example

* test for upgradeable contracts (#203)

* added erc20 unit test

* added test against custom errors

* formated

* made requested changes and moved test to contract module

* fixed zero address

* chore: Remove unused test files

* test for upgradeable contracts

* fixed build issue

* fix: pr#203

---------

Co-authored-by: julio4 <[email protected]>

* Components chapter: wording, grammar, formatting fixes (#210)

* how-to fixes

* Clean up dependencies

* collisions > fixes

* ownable minor fixes

* remove error.log

* fix typo

* add comma after 'first'

* add 'the' before cairo book

---------

Co-authored-by: Nenad <[email protected]>

* Advanced concepts: typos, wording, grammar, formatting (#215)

* formatting for write to slot

* stor. arrs. > wording, grammar

* hashing > wording, grammar

* packing > wording, grammar, format

* list > amount->amounts, wording, typos

* plugins > grammar

* sign. verif. > grammar

* remove 'a' in how_to

* update library calls -> dispatcher

* Revert "update library calls -> dispatcher"

This reverts commit e7d3b0c.

* align library calls header

* fix tests

---------

Co-authored-by: Nenad <[email protected]>

* feat: nft dutch auction (#204)

* feat: nft auction

* test: add tests to nft_auction app

* chore: improve code and add more tests

* chore: improvements and more tests

* test: add more test cases for nft_auction

* chore: update mdbook

* chore: update nft_auction package
- Add error module
- Update snforge version to 0.24.0

* chore: rename package and related files from `nft_auction` to `nft_dutch_auction`

* chore: reused existing package

* fix: cli#204

---------

Co-authored-by: julio4 <[email protected]>

* fix: chapter-related folder names (#216)

* fix: chapter-related folder names

* updated all references

* feat: simple storage with starknet-js (#222)

* feat: simple storage with starknet-js

* feat: add how_to_deploy & fix tutorial content for simple_storage

* fix: update links & add section in summary

* feat: revisions #222

---------

Co-authored-by: julio4 <[email protected]>

* chore: scarb, foundry, oz updates (#227)

* feat: Advanced factory contract (#219)

* add initial factory

* add ownable component

* add caller to CounterCreated event

* turn counter into campaign

* fix Campaign interfaced funcs + implement donate

* add _assert_is_ended + update error messages

* _assert_active->_assert_campaign_active

* _assert_is_ended->_assert_campaign_ended

* implement withdraw

* add missing assert success in donate

* add title & description

* update comment

* implement upgrade

* clean up internal funcs and imports

* move hardcoded errors in Errors mod

* donate -> contribute + event rename

* withdraw -> claim

* add store impl for contract addr. array

* remove store impl

* add dynamic array impl

* remove dyn. array

* remove descr + convert title to felt + convert target to u128

* implement updating class hashes

* Make title ByteArray again + target into u256 + update ctor arg serialization

* refactor serialization + add back description

* remove unused contracts

* add 1 test

* add get_description

* add correct deps

* add alexandria to toml

* format factory.cairo

* add missing snforge workspace

* add missing getters + tests

* add factory deploy tests

* add class hash update test + event assertions

* assert old class hash prior to update

* remove commented out test

* use common alex. storage workspace in using_lists

* add missing newline in toml

* move factory tests to separate file

* add scaffold docs for contracts

* add end_time asserts

* refactor private asserts

* check if target reached before claiming

* add ability to withdraw funds

* make contributions into a component (now iterable)

* refactor 'withhold' - contrs map to amt_idx

* add get_contributors func

* get_contributors -> get_contributions

* total_contributors->contributor_count

* add tests for campaign upgrade and deploy + update all relevant code in factory

* add status to campaign

* add close fn

* pass desired donation token in ctor

* merge all getters into get_details

* return total_contributions in details

* remove rev version from alexandria dep

* verbose names

* reorg. folder structure

* add tag to alexandria dep

* campaign_upgrade.cairo->mock_upgrade.cairo

* add explicit alexandria rev + make crowdfunding contracts standalone chapters

* add status pending

* field rename: factory->creator

* refund users when upgrading campaign

* Make owner the calling address, and creator is the campaign manager

* add get_contributor (amount) func

* Add successful campaign test

* update comment for upgrade

* _refund_all->_withdraw_all

* update checks for withdraw

* rework contribute

* rework all funcs

* unsuccessful -> failed

* calc end_time in start fn

* calc end_time in upgrade fn

* makes upgrades callable only by creators in factory

* fix factory tests

* fix crowdfunding tests

* reduce total contri. when withdraw from act. camp

* add refund fn

* refactor withdraw_all to use _refund

* pending->draft

* fix mock and tests

* add test for close

* add test for withdraw

* upgrade > update end_time only if duration provided

* close->cancel

* rename to more align with Solidity by example

* target->goal

* remove comment

* err CLOSED->CANCELED + check active in unpledge

* contributor->pledger

* add campaign doc content

* remove draft status

* add start_time

* remove Status

* update doc for campaign

* move total_pledges to pledgeable

* reorder alphabetically

* remove Launched event + upgrade mock

* TARGET->GOAL

* reorder params in Details

* add inline to _refund

* add new pledgeable tests

* add getX tests + add get_pledge_count

* refactor pledger_to_amount_index->pledger_to_amount

* Add tests with 1000 pledgers

* add test for add + update existing pledger

* reenable lib

* Add link to adv. factory in crowdfunding point 9

* write the adv. factory chapter

* upgrade_campaign_implementation-> upgrade_campaign + comment updates

* rename get_pledgers_as_arr->array

* Use ERC20Upgradeable instead of ERC20 preset

* Add missing token recipient ctor argument in crowdfunding tests

---------

Co-authored-by: Nenad <[email protected]>

* test: countable (switchable, ownable) components (#205)

* test: implement test for countable component

* test: implement test for switchable component

* test: implement test for ownable component

* chore: implement test for emitted events and add anchor tags

* feat: apply requested changes

* feat: apply requested changes

* feat: revisions

---------

Co-authored-by: julio4 <[email protected]>

* doc: SNIP-6 implementation (#200)

* test: SNIP-6 implementation

* docs: errors recheck

* test: SNIP-6 implementation

* docs: errors recheck

* feat: add simple account example

* feat/fix: revisions on #200

* feat:implement SRC5

* feat: implementation with oz

* fix: oz impl src5 for account

---------

Co-authored-by: Oluwaseun Jeremiah <jeremiah@Jemiah>
Co-authored-by: julio4 <[email protected]>

* simple_vault test implementation (#220)

* Update es.po (#231)

Some updates and corrections.

* Add Dict Cheatsheet (#235)

* simple_vault test implementation

* Added dict cheatsheet

* Added dict cheatsheet

* a little clean up

* a little clean up

* Mention Foundry is an option for tests (#240)

* Update SUMMARY.md (#239)

- Add link to NFT Dutch Auction chapter

* fix: links, typos, preludes... (#241)

* Update Cairo >2.8, `2024_07` edition (#246)

* remove List

No longer needed with Vec

* chore: dependencies update >=2.8

* chore: 2024_07 edition getting-started

* chore: Map getting-started

* chore: update applications

* chore: update dependencies

* fix: storage variables PointerReadAccess

* doc: update contributor guide

* chore: update advanced-concepts

* fix: 2024_07 edition

* chore: versions update

* feat: storage custom types individual members access

* remove storing arrays chapter

* ci/cd: remove custom test resolver script

* chore: switch dependencies to scarb registry

* Revert "ci/cd: remove custom test resolver script"

This reverts commit 0c3549f.

* chore: split snforge/cairo-test scarb config

* Random Number Generator (#238)

* feat: dice game vrf application

* feat: add summary nav

* fix: ran scarb fmt

* fix: ran scarb fmt

* Fix new lines

* Add more info on randomness sources

* Rename dice_game_vrf.md->random_number_generator.md and update titles

* minor rewording of 1 entropy source

* remove anchors

* Minor changes to fn names

* Implement dice game scaffold

* Implement Pragma randomness

* minor refactor in randomness request

* Implement powerball scaffold

* Turn Dice Game into CoinFlip

* Implement coin_flip test

* Add more tests

* Update titles

* Remove redundant blank line

* Add premium fee calculation into tests

* Assert leftover balance

* Remove comment about fees

* Increase the expected callback fee, update mock to expose fee calc fn

* Unfinished: refunded

* Store and use is_refunded flag

* Implement logic necessary to successfully perform & test refund

* Update callback fee limit based on manual testing + update term to deposit

* Format

* Use a FlipData struct instead of tuple

* Fix refund

* Simplify CoinFlip to pay the flips itself

* CALLBACK_FEE_DEPOSIT->MAX_CALLBACK_FEE_DEPOSIT

* Update tests to test the new CoinFlip contract

* Fix compile errors

* Increase publish_delay to 1 & remove unused imports

* Remove starkli-wallet dir

* Generate 3 random words for the 1st test

* refactor tests

* Add missng newline to scarb.toml

* fix typo in md

* reword 'manipulation' def

* Chainlink->Pragma

* link to Commit-reveal chapter issue

* list 'shut down' as possible centr. issue with ext. oracles

* Turn point 5 into a note

* Remove Sideways enum

* add contract description

* Remove ResultTrait from crowdfunding tests.cairo

---------

Co-authored-by: Tony Stark <[email protected]>
Co-authored-by: Nenad <[email protected]>
Co-authored-by: Nenad <[email protected]>

* feat(merkle-tree): Contract with tests (#228)

* feat(merkle-tree): Contract with tests

* feat(merkle-tree): Corrections according to PR reviews

* feat(merkle-tree): Contract with tests

* fix: 2024_07 edition

* fix: Replace Map simulating Array with Vec - streamline md file explanations

* fix: scarb fmt

---------

Co-authored-by: julio4 <[email protected]>

* Improvement: typos check in CI/CD (#248)

* fix: typos

* ci/cd: typos check in ci

* fix: cases

* fix: randomness requestor typos

* feat: cairo syntax hl (#249)

* Expand Constant Product AMM's description (#252)

* Expand Constant Product AMM's description

* Refactor

---------

Co-authored-by: Julio <[email protected]>

* Minor _Scarb.toml_ refactors (#251)

* Set one common edition version in workspace Scarb.toml

* move snforge_std to dev-deps

* Remove [lib] from coin_flip and simple_storage_starknetjs

* rebuild projects

* remove casm attr from scarb.toml

* Update foundry to 0.31.0 in tool-versions

* Set workspace cairo_test in mappings

* Add missing cairo_test.workspace attrbs to Scarb.tomls

* Revert snforge_std to 0.30.0

* Revert "Add missing cairo_test.workspace attrbs to Scarb.tomls"

This reverts commit 7a9131d.

* scarb.lock

* feat: commit-reveal pattern

---------

Co-authored-by: the-first-elder <[email protected]>
Co-authored-by: julio4 <[email protected]>
Co-authored-by: Okoli Evans <[email protected]>
Co-authored-by: Nenad Misić <[email protected]>
Co-authored-by: Nenad <[email protected]>
Co-authored-by: Asher <[email protected]>
Co-authored-by: hudem1 <[email protected]>
Co-authored-by: saimeunt <[email protected]>
Co-authored-by: princeibs <[email protected]>
Co-authored-by: Wolf <[email protected]>
Co-authored-by: Ege <[email protected]>
Co-authored-by: Jules Doumeche <[email protected]>
Co-authored-by: Nenad Misić <[email protected]>
Co-authored-by: Emmanuel A Akalo <[email protected]>
Co-authored-by: Jemiiah <[email protected]>
Co-authored-by: Oluwaseun Jeremiah <jeremiah@Jemiah>
Co-authored-by: Orlando Sanchez <[email protected]>
Co-authored-by: Tony Stark <[email protected]>
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.

3 participants