Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

Add alt_bn128 syscall tests (and fix related issues) #31436

Merged
merged 1 commit into from
May 22, 2023
Merged

Add alt_bn128 syscall tests (and fix related issues) #31436

merged 1 commit into from
May 22, 2023

Conversation

vadorovsky
Copy link
Contributor

@vadorovsky vadorovsky commented May 2, 2023

  • Fix C headers (the name of the syscall was incorrect).
  • Add C SBF tests using the alt_bn128 syscall.
  • Fix the Rust SBF program:
    • Do not use serde and array-bytes, provide test cases as byte arrays directly.
    • Use the custom_heap_default macro.
    • Replace bpf with sbf in the crate name.
  • Execute both previously existing Rust tests and new C tests in programs/sbf/tests, so they are actually tested on CI.

/cc @Lichtso @ananas-block

@mergify mergify bot added community Community contribution need:merge-assist labels May 2, 2023
@mergify mergify bot requested a review from a team May 2, 2023 02:49
@vadorovsky vadorovsky changed the title Add alt_bn128 syscall tests Add alt_bn128 syscall tests (and fix related issues) May 2, 2023
@CriesofCarrots CriesofCarrots requested a review from Lichtso May 10, 2023 18:38
@codecov
Copy link

codecov bot commented May 22, 2023

Codecov Report

Merging #31436 (0ba58ff) into master (bc4d53c) will increase coverage by 0.0%.
The diff coverage is n/a.

❗ Current head 0ba58ff differs from pull request most recent head 5da6c1e. Consider uploading reports for the commit 5da6c1e to get more accurate results

@@           Coverage Diff           @@
##           master   #31436   +/-   ##
=======================================
  Coverage    81.8%    81.8%           
=======================================
  Files         736      736           
  Lines      205955   205932   -23     
=======================================
+ Hits       168631   168651   +20     
+ Misses      37324    37281   -43     

Lichtso
Lichtso previously approved these changes May 22, 2023
@Lichtso
Copy link
Contributor

Lichtso commented May 22, 2023

@CriesofCarrots We need one more approval on this.

CriesofCarrots
CriesofCarrots previously approved these changes May 22, 2023
Copy link
Contributor

@CriesofCarrots CriesofCarrots left a comment

Choose a reason for hiding this comment

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

Merge commit 😞

@Lichtso
Copy link
Contributor

Lichtso commented May 22, 2023

@vadorovsky Can you delete the last two commits and rebase?

@vadorovsky vadorovsky dismissed stale reviews from CriesofCarrots and Lichtso via 4f5649e May 22, 2023 19:29
@mergify mergify bot requested a review from a team May 22, 2023 19:29
Lichtso
Lichtso previously approved these changes May 22, 2023
* Fix C headers (the name of the syscall was incorrect).
* Add C SBF tests using the alt_bn128 syscall.
* Fix the Rust SBF program:
  * Do not use serde and array-bytes, provide test cases as byte arrays
    directly.
  * Use the `custom_heap_default` macro.
  * Replace `bpf` with `sbf` in the crate name.
* Execute both previously existing Rust tests and new C tests in
  `programs/sbf/tests`, so they are actually tested on CI.
@vadorovsky
Copy link
Contributor Author

@Lichtso @CriesofCarrots done, thank you for quick reaction!

@mvines mvines added the CI Pull Request is ready to enter CI label May 22, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label May 22, 2023
@mvines
Copy link
Contributor

mvines commented May 22, 2023

CI is green, PR is approved. Doing a drive-by merge-assist on this.

@mvines mvines merged commit bea062b into solana-labs:master May 22, 2023
@vadorovsky vadorovsky deleted the test-alt_bn128 branch May 22, 2023 22:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community Community contribution need:merge-assist
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants