Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Better tests for keccak precompile #58

Merged
merged 8 commits into from Nov 21, 2023

Conversation

StanislavBreadless
Copy link
Collaborator

What ❔

Use the current bytecode in the tests of the keccak precompile and basic randomized inputs

Why ❔

Checklist

  • PR title corresponds to the body of PR (we generate changelog entries from PRs).
  • Tests for the changes have been added / updated.
  • Documentation comments have been added / updated.
  • Code has been formatted via zk fmt and zk lint.

@@ -39,6 +50,27 @@ describe('Keccak256 tests', function () {
await keccakTest.zeroPointerTest()
});

it('general functionality test', async () => {
// We currently do not have fussing support, so we generate random data using
Copy link
Member

Choose a reason for hiding this comment

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

Agree that it is non-trivial to integrate Echidna or other Fuzz tooling at the moment. But this test seems to me weird. data is always 32 bytes long. I would rather make a couple of randomly generated tests:

  • 0 bytes long
  • x * BLOCK_SIZE - 2
  • x * BLOCK_SIZE - 1
  • x * BLOCK_SIZE
  • x * BLOCK_SIZE + 1
  • x * BLOCK_SIZE + 2

It is to cover all possible variants of branching

@koloz193
Copy link
Contributor

build is likely failing due to the fact that the artifact folder doesnt exist when we run preprocessor. we could likely fix by compiling -> preprocessing -> compiling but IMO it also seems like overkill

@StanislavBreadless StanislavBreadless merged commit 123bb68 into v1-4-1-integration Nov 21, 2023
5 of 6 checks passed
@StanislavBreadless StanislavBreadless deleted the sb-enhance-keccak-tests branch November 21, 2023 16:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants