equihash: Add Rust API for Tromp solver #1083

str4d · 2024-01-04T04:42:20Z

No description provided.

components/equihash/tromp/equi_miner.c

components/equihash/src/lib.rs

codecov · 2024-01-04T19:08:43Z

Codecov Report

Attention: Patch coverage is 57.37705% with 26 lines in your changes missing coverage. Please review.

Project coverage is 54.37%. Comparing base (0432867) to head (f3d5a5c).
Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
components/equihash/src/tromp.rs	0.00%	26 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1083   +/-   ##
=======================================
  Coverage   54.36%   54.37%           
=======================================
  Files         175      176    +1     
  Lines       20325    20385   +60     
=======================================
+ Hits        11050    11084   +34     
- Misses       9275     9301   +26

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

str4d · 2024-04-18T20:07:58Z

Force-pushed to clean up the commit history; no changes to @teor2345's changes, just regrouping them (and squashing some of the fixes into the original commits that didn't make sense to keep in standalone commits).

str4d · 2024-04-18T20:14:06Z

Force-pushed to GitHub-linkify an imported source link in a commit message.

str4d · 2024-04-18T20:14:31Z

And somehow I managed to click two wrong buttons in a row 🤦

str4d · 2024-04-18T20:10:04Z

components/equihash/src/tromp.rs

-        .collect()
+        .collect();
+
+    // Just in case the solver returns solutions that become the same when compressed.


I don't think this should ever be the case. The compressed representation should not be lossy, it just packs the indices together to remove the effective padding bits that exist in the u32 representation.

~~It may not have been observed, or there could be a subtle difference between the C code and the Rust code in compress_array().~~

Update: Yeah, something must be wrong if identical solutions were observed, I'll check for changes to the logic in equi_miner.c, everything else looks equivalent to the zcashd logic.

Update: Duplicate solutions were observed prior to a bug fix, is there any chance that the solver could produce duplicate uncompressed solutions?

str4d · 2024-04-18T20:11:35Z

components/equihash/src/tromp.rs

@@ -70,13 +71,21 @@ unsafe fn worker(eq: *mut CEqui, p: verify::Params, curr_state: &State) -> Vec<V
        let nsols = equi_nsols(eq);
        let sols = equi_sols(eq);
        let solution_len = 1 << p.k;
+        //println!("{nsols} solutions of length {solution_len} at {sols:?}");


I'm inclined to remove this and the other introduced debugging print statements, unless anyone else disagrees.

I think we should keep the debug print statements and consider adding more.

str4d · 2024-04-18T20:53:41Z

Force-pushed to clean up the feature flagging in the build script, and add C #ifdefs to prevent some warnings.

str4d · 2024-04-18T23:21:59Z

Rebased on current main. I will rebase this PR again once #1357 merges.

Source: zcash/zcash@01d5576 License: MIT

Co-authored-by: teor <teor@riseup.net>

This avoids linker errors by removing cycles.

Co-authored-by: teor <teor@riseup.net>

Source: mikepb/endian.h@0f885cb License: Public Domain (or "BSD OR MIT OR Apache-2.0")

The equivalent change is made to the C worker, which is unused.

str4d · 2024-04-19T02:29:00Z

Force-pushed to adjust the minimal-form compression method APIs and integrate them into the existing tests.

components/equihash/src/tromp.rs

arya2

This looks like it should work so far, and none of my suggestions are blockers, feel free to resolve any/all of them without making changes.

I'm still reviewing equi_miner.c in detail, but everything else looks equivalent to the logic in zcashd, at least for the Mainnet parameters.

Update: Collision errors are still being logged here (and DuplicateIdxs errors).

components/equihash/tromp/equi.h

components/equihash/tromp/portable_endian.h

components/equihash/src/minimal.rs

arya2 · 2024-07-05T00:09:49Z

components/equihash/src/minimal.rs

+                )
+                    .wrapping_shl(8 * (in_width - x - 1) as u32); // Big-endian


This seems like an unnecessary change from the c++ logic, it should panic in minimal_from_indices() before it gets here when using parameters that would be affected by the change.

Suggested change

)

.wrapping_shl(8 * (in_width - x - 1) as u32); // Big-endian

) << (8 * (in_width - x - 1) as u32); // Big-endian

Is it possible this is a problem if it's different than what the original code was doing?

components/equihash/src/minimal.rs

components/equihash/src/tromp.rs

components/equihash/tromp/equi_miner.c

arya2

@str4d Testing indicates that there are still some issues in equi_miner.c, but I think we should document this as an experimental feature and otherwise merge it as-is, because it usually works, the occasional unexpected errors are handled gracefully, the internal-miner feature in Zebra that uses it is already documented as experimental, we need to merge it to publish a version of zebrad that could support a Zebra-only Testnet, and zcashd is being deprecated soon enough that de-duplicating the code here with what zcashd is using may not be worthwhile.

We could try porting it over to Rust or fixing the occasional issues in the current C code once the resource-constrained steps needed to deploy NU7 are complete. Running this code via Zebra could produce some helpful debug output if we keep the debug print statements and perhaps add more of them.

If we don't want to document it as an experimental feature, a pair review of equi_miner.c could be helpful for figuring out what's causing the Collision and DuplicateIdxserrors.

components/equihash/tromp/equi_miner.c

components/equihash/Cargo.toml

Co-authored-by: Arya <aryasolhi@gmail.com>

mpguerra · 2024-11-14T09:06:44Z

Do we want to merge this one? It's blocking us from restoring the internal-miner feature in zebra

str4d

utACK f3d5a5c. I've made the changes I wanted for the release; the remaining issues are non-blocking and can be fixed over time.

arya2

Thank you!!

str4d commented Jan 4, 2024

View reviewed changes

components/equihash/tromp/equi_miner.c Show resolved Hide resolved

str4d commented Jan 4, 2024

View reviewed changes

components/equihash/src/lib.rs Outdated Show resolved Hide resolved

str4d force-pushed the equihash-solver-tromp branch from cb60b5b to f3ea80e Compare January 4, 2024 18:47

teor2345 mentioned this pull request Jan 8, 2024

feat(mine): Add an internal Zcash miner to Zebra ZcashFoundation/zebra#8136

Merged

11 tasks

bishopcheckmate mentioned this pull request Jan 16, 2024

Add Zebra crate versions to dev-dependencies, to enable crater tests ZcashFoundation/zebra#6924

Closed

3 tasks

This was referenced Jan 22, 2024

Remove or comment out zebra internal miner ZcashFoundation/zebra#8180

Closed

Restore internal miner ZcashFoundation/zebra#8183

Open

str4d mentioned this pull request Jan 26, 2024

equihash: Fix some Rust API issues #1088

Merged

daira assigned str4d Mar 14, 2024

str4d force-pushed the equihash-solver-tromp branch from 8e66e0d to d33ec1d Compare April 18, 2024 20:06

str4d force-pushed the equihash-solver-tromp branch from d33ec1d to 9dbdf18 Compare April 18, 2024 20:13

str4d closed this Apr 18, 2024

str4d deleted the equihash-solver-tromp branch April 18, 2024 20:14

str4d restored the equihash-solver-tromp branch April 18, 2024 20:14

str4d reopened this Apr 18, 2024

str4d commented Apr 18, 2024

View reviewed changes

str4d force-pushed the equihash-solver-tromp branch from 9dbdf18 to b201c10 Compare April 18, 2024 20:52

str4d mentioned this pull request Apr 18, 2024

Refactor equihash code #1357

Merged

str4d force-pushed the equihash-solver-tromp branch from b201c10 to be16706 Compare April 18, 2024 23:21

str4d and others added 5 commits April 19, 2024 02:00

equihash: Import Tromp solver

45652a2

Source: zcash/zcash@01d5576 License: MIT

equihash: Import blake2b_simd C bindings from zcashd

7ab6c47

Source: zcash/zcash@01d5576 License: MIT

equihash: Modify Tromp solver to compile as C

3aaeb8b

Co-authored-by: teor <teor@riseup.net>

equihash: Pass blake2b_simd bindings to Tromp solver as callbacks

45e7238

This avoids linker errors by removing cycles.

equihash: Add Rust API for Tromp solver

d07505d

Co-authored-by: teor <teor@riseup.net>

teor2345 added 7 commits April 19, 2024 02:27

equihash: Place solver behind a feature flag

5f77bd7

equihash: Ensure returned compressed solutions are unique

d7ccd07

equihash: Add a portable endian.h for htole32() on macOS and Windows

989f40e

Source: mikepb/endian.h@0f885cb License: Public Domain (or "BSD OR MIT OR Apache-2.0")

equihash: Remove unused thread support to enable Windows compilation

b737d0f

equihash: Don't import a header that's missing in Windows CI

2bd7bc8

equihash: Clear slots when setting the hash state

9391e65

The equivalent change is made to the C worker, which is unused.

Add commented-out prints of solution candidates for debugging

76131db

str4d force-pushed the equihash-solver-tromp branch from 6571aa9 to 76131db Compare April 19, 2024 02:28

str4d commented Apr 19, 2024

View reviewed changes

components/equihash/src/tromp.rs Outdated Show resolved Hide resolved

str4d marked this pull request as ready for review April 19, 2024 02:34

arya2 self-requested a review June 20, 2024 15:05

arya2 reviewed Jul 5, 2024

View reviewed changes

arya2 mentioned this pull request Oct 2, 2024

add(mining): Restore internal miner ZcashFoundation/zebra#8906

Closed

arya2 reviewed Oct 18, 2024

View reviewed changes

components/equihash/tromp/equi_miner.c Show resolved Hide resolved

components/equihash/Cargo.toml Outdated Show resolved Hide resolved

Note in Cargo.toml that this crate is experimental

634285d

Co-authored-by: Arya <aryasolhi@gmail.com>

arya2 previously approved these changes Nov 1, 2024

View reviewed changes

str4d added 4 commits February 13, 2025 02:05

Merge branch 'main' into equihash-solver-tromp

aed3ecb

equihash: Only expose a solver for compressed solutions

6c7cce2

equihash: Update changelog

e8e3e87

equihash: Remove BLAKE2b bindings from public API

f3d5a5c

str4d dismissed arya2’s stale review via f3d5a5c February 13, 2025 02:09

str4d commented Feb 13, 2025

View reviewed changes

mpguerra requested review from teor2345 and removed request for teor2345 February 13, 2025 09:07

nuttycom requested a review from arya2 February 13, 2025 14:08

arya2 approved these changes Feb 13, 2025

View reviewed changes

str4d merged commit 094e0eb into main Feb 14, 2025
35 checks passed

str4d deleted the equihash-solver-tromp branch February 14, 2025 02:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

equihash: Add Rust API for Tromp solver #1083

equihash: Add Rust API for Tromp solver #1083

str4d commented Jan 4, 2024

codecov bot commented Jan 4, 2024 •

edited

Loading

str4d commented Apr 18, 2024

str4d commented Apr 18, 2024

str4d commented Apr 18, 2024

str4d Apr 18, 2024

arya2 May 4, 2024 •

edited

Loading

str4d Apr 18, 2024

arya2 Oct 18, 2024

str4d commented Apr 18, 2024

str4d commented Apr 18, 2024

str4d commented Apr 19, 2024

arya2 left a comment •

edited

Loading

arya2 Jul 5, 2024 •

edited

Loading

arya2 Nov 1, 2024

arya2 left a comment •

edited

Loading

mpguerra commented Nov 14, 2024

str4d left a comment

arya2 left a comment

		)
		.wrapping_shl(8 * (in_width - x - 1) as u32); // Big-endian

	)
	.wrapping_shl(8 * (in_width - x - 1) as u32); // Big-endian
	) << (8 * (in_width - x - 1) as u32); // Big-endian

equihash: Add Rust API for Tromp solver #1083

equihash: Add Rust API for Tromp solver #1083

Conversation

str4d commented Jan 4, 2024

codecov bot commented Jan 4, 2024 • edited Loading

Codecov Report

str4d commented Apr 18, 2024

str4d commented Apr 18, 2024

str4d commented Apr 18, 2024

str4d Apr 18, 2024

Choose a reason for hiding this comment

arya2 May 4, 2024 • edited Loading

Choose a reason for hiding this comment

str4d Apr 18, 2024

Choose a reason for hiding this comment

arya2 Oct 18, 2024

Choose a reason for hiding this comment

str4d commented Apr 18, 2024

str4d commented Apr 18, 2024

str4d commented Apr 19, 2024

arya2 left a comment • edited Loading

Choose a reason for hiding this comment

arya2 Jul 5, 2024 • edited Loading

Choose a reason for hiding this comment

arya2 Nov 1, 2024

Choose a reason for hiding this comment

arya2 left a comment • edited Loading

Choose a reason for hiding this comment

mpguerra commented Nov 14, 2024

str4d left a comment

Choose a reason for hiding this comment

arya2 left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 4, 2024 •

edited

Loading

arya2 May 4, 2024 •

edited

Loading

arya2 left a comment •

edited

Loading

arya2 Jul 5, 2024 •

edited

Loading

arya2 left a comment •

edited

Loading