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: adds tests and fixes to home contract #38

Merged
merged 15 commits into from
Feb 4, 2021
Merged

Conversation

luketchang
Copy link
Contributor

@luketchang luketchang commented Feb 3, 2021

Currently covers the following:

  • suggest update behavior
  • halt on fail
  • double updates
  • update acceptance
  • improper updates

TODO:

  • root calculation and the root queue (test for Merkle test suite?)

Questions:

  • Still figuring out testing style; which repeated setups should I extract to helper functions and where to put them? How to make tests cleaner overall?
  • It doesn't look like current is ever updated to newRoot; this is mistake?
  • Why do double updates in particular warrant their own fraud proof function?

Related to #15

@luketchang luketchang self-assigned this Feb 3, 2021
@luketchang luketchang added solidity ♦️ Solidity dev work required bug Something isn't working testing Unit, integrations, and e2e testing labels Feb 3, 2021
anna-carroll
anna-carroll previously approved these changes Feb 3, 2021
Copy link
Contributor

@anna-carroll anna-carroll left a comment

Choose a reason for hiding this comment

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

lookin goood 🤠

solidity/test/Home.test.js Outdated Show resolved Hide resolved
solidity/contracts/Common.sol Show resolved Hide resolved
solidity/contracts/Queue.sol Show resolved Hide resolved
solidity/test/Home.test.js Outdated Show resolved Hide resolved
solidity/test/Home.test.js Outdated Show resolved Hide resolved
solidity/contracts/test/TestHome.sol Outdated Show resolved Hide resolved
solidity/test/Home.test.js Outdated Show resolved Hide resolved
solidity/test/Home.test.js Outdated Show resolved Hide resolved
solidity/test/Home.test.js Outdated Show resolved Hide resolved
solidity/test/Home.test.js Outdated Show resolved Hide resolved
solidity/test/Home.test.js Outdated Show resolved Hide resolved
Copy link
Contributor

@prestwich prestwich left a comment

Choose a reason for hiding this comment

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

1 small layout request

solidity/test/Home.test.js Outdated Show resolved Hide resolved
@luketchang luketchang linked an issue Feb 3, 2021 that may be closed by this pull request
10 tasks
@prestwich prestwich merged commit e89ab4c into main Feb 4, 2021
@prestwich prestwich deleted the luke/home-tests branch February 4, 2021 16:22
prestwich pushed a commit that referenced this pull request Feb 7, 2021
* lint: run prettier on js tests and scripts

* tests: add 100% test coverage for the queue contract

* test: adds halt on failed state and in progress test on accepting updates

* fix: Updater.signUpdate now hashes message with Ethereum prefix before signing

* fix: fixed off-by-one error in queue contains() where for loop wasn't visiting _q.last

* fix: fix equality logic for double update check in Common.sol

* feature: adds tests for skipping current root and submitting update with non-existent new root

* test: adds test that ensures rejection of updates from fake updater

* test: adds double update test

* refactor: adds enqueueMessageAndSuggestUpdate helper function to shorten tests

* test: adds test for suggested updates

* refactor: changes doubleUpdate function to take single oldRoot rather than array

* refactor: Home test helper function enqueues message and returns its root rather than both current and new root

* refactor: moved queueContains and queueEnd from TestHome to QueueManager contract

* refactor: moves signer and updater initialization to before block, now updater creation uses async static fromSigner

Co-authored-by: James Prestwich <prestwich@clabs.co>
luketchang added a commit that referenced this pull request Feb 8, 2021
* lint: run prettier on js tests and scripts

* tests: add 100% test coverage for the queue contract

* test: adds halt on failed state and in progress test on accepting updates

* fix: Updater.signUpdate now hashes message with Ethereum prefix before signing

* fix: fixed off-by-one error in queue contains() where for loop wasn't visiting _q.last

* fix: fix equality logic for double update check in Common.sol

* feature: adds tests for skipping current root and submitting update with non-existent new root

* test: adds test that ensures rejection of updates from fake updater

* test: adds double update test

* refactor: adds enqueueMessageAndSuggestUpdate helper function to shorten tests

* test: adds test for suggested updates

* refactor: changes doubleUpdate function to take single oldRoot rather than array

* refactor: Home test helper function enqueues message and returns its root rather than both current and new root

* refactor: moved queueContains and queueEnd from TestHome to QueueManager contract

* refactor: moves signer and updater initialization to before block, now updater creation uses async static fromSigner

Co-authored-by: James Prestwich <prestwich@clabs.co>
luketchang added a commit that referenced this pull request Feb 8, 2021
* lint: run prettier on js tests and scripts

* tests: add 100% test coverage for the queue contract

* test: adds halt on failed state and in progress test on accepting updates

* fix: Updater.signUpdate now hashes message with Ethereum prefix before signing

* fix: fixed off-by-one error in queue contains() where for loop wasn't visiting _q.last

* fix: fix equality logic for double update check in Common.sol

* feature: adds tests for skipping current root and submitting update with non-existent new root

* test: adds test that ensures rejection of updates from fake updater

* test: adds double update test

* refactor: adds enqueueMessageAndSuggestUpdate helper function to shorten tests

* test: adds test for suggested updates

* refactor: changes doubleUpdate function to take single oldRoot rather than array

* refactor: Home test helper function enqueues message and returns its root rather than both current and new root

* refactor: moved queueContains and queueEnd from TestHome to QueueManager contract

* refactor: moves signer and updater initialization to before block, now updater creation uses async static fromSigner

Co-authored-by: James Prestwich <prestwich@clabs.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working solidity ♦️ Solidity dev work required testing Unit, integrations, and e2e testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write tests for the Home contract
3 participants