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

Implement Compilation tests with 100% coverage #1856

Merged
merged 7 commits into from
Jan 23, 2025

Conversation

marcocastignoli
Copy link
Member

@marcocastignoli marcocastignoli commented Jan 16, 2025

TODO

  • Vyper tests 100% coverage
  • Vyper test refactoring
  • Solidity tests 100% coverage
  • Solidity test refactoring

@marcocastignoli marcocastignoli self-assigned this Jan 20, 2025
@marcocastignoli marcocastignoli marked this pull request as ready for review January 22, 2025 09:17
Copy link
Contributor

@manuelwedler manuelwedler left a comment

Choose a reason for hiding this comment

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

A couple of questions and minor findings.

@@ -112,17 +111,6 @@ export class SolidityCompilation extends AbstractCompilation {

const auxdataFromRawRuntimeBytecode = `${runtimeAuxdataCbor}${runtimeCborLengthHex}`;

// For some reason the auxdata from raw bytecode differs from the legacyAssembly's auxdata
if (auxdatasFromCompilerOutput[0] !== auxdataFromRawRuntimeBytecode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you explain why we don't need to handle this case anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

This would be a problem of the compiler. I don't think we have to handle this case.

Here we are not checking the auxdata against onchain bytecode, but recompilied bytecode. So the only case in which something like this would happen is if the compiler writes an auxdata in the legacyAssembly that is not the same as the one in the bytecode.

marcocastignoli and others added 3 commits January 23, 2025 10:34
Co-authored-by: Manuel Wedler <34456797+manuelwedler@users.noreply.github.com>
Co-authored-by: Manuel Wedler <34456797+manuelwedler@users.noreply.github.com>
Copy link
Contributor

@manuelwedler manuelwedler left a comment

Choose a reason for hiding this comment

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

Looks good to me

@marcocastignoli marcocastignoli merged commit d1491e1 into verification-flow-refactoring-main Jan 23, 2025
6 checks passed
@marcocastignoli marcocastignoli deleted the compilation-tests branch January 23, 2025 10:13
kuzdogan pushed a commit that referenced this pull request Feb 28, 2025
* push vyper tests (still need refactoring)

* Implement solidity test to get 100% coverage

* Refactor Vyper tests

* refactor solidity tests

* Update packages/lib-sourcify/src/Compilation/VyperCompilation.ts

Co-authored-by: Manuel Wedler <34456797+manuelwedler@users.noreply.github.com>

* Update packages/lib-sourcify/test/Compilation/VyperCompilation.spec.ts

Co-authored-by: Manuel Wedler <34456797+manuelwedler@users.noreply.github.com>

* fix duplicate check

---------

Co-authored-by: Manuel Wedler <34456797+manuelwedler@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
Status: COMPLETED
Development

Successfully merging this pull request may close these issues.

2 participants