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

fix(rln): atomic operation edge case #195

Merged
merged 12 commits into from
Aug 1, 2023
Merged

Conversation

rymnc
Copy link
Contributor

@rymnc rymnc commented Jul 31, 2023

Tracking from waku-org/go-zerokit-rln#12

This PR improves readability of the override_range fn, by matching on 4 input cases, and handles all of them
appropriately.

@rymnc rymnc added the track:rln RLN Track - (Secure Messaging/Applied ZK), relay and applications label Jul 31, 2023
@rymnc rymnc self-assigned this Jul 31, 2023
@rymnc rymnc requested a review from richard-ramos July 31, 2023 08:02
@rymnc rymnc force-pushed the fix-atomic-operation-edge-case branch from 197b808 to 7f0ac87 Compare July 31, 2023 08:09
@github-actions
Copy link

Benchmark for 267d8c1

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1256.5±1.77ns 1257.8±10.33ns +0.10%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.1±0.00µs 3.1±0.01µs 0.00%
FullMerkleTree::set 1250.5±1.59ns 1249.1±1.53ns -0.11%
OptimalMerkleTree::compute_root 1390.5±1.44ns 1390.1±1.12ns -0.03%
OptimalMerkleTree::delete 1388.2±1.26ns 1383.8±2.56ns -0.32%
OptimalMerkleTree::get 29.8±0.02ns 29.1±0.11ns -2.35%
OptimalMerkleTree::override_range 7.0±0.01µs 7.0±0.01µs 0.00%
OptimalMerkleTree::set 1393.5±14.64ns 1389.0±4.09ns -0.32%

@github-actions
Copy link

Benchmark for 267d8c1

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.4±0.00ns 2.0±0.00ns -16.67%
Pmtree::get 502.2±5.10ns 469.3±0.45ns -6.55%
Pmtree::override_range 170.3±4.69µs 174.9±6.89µs +2.70%
Pmtree::set 72.4±0.22µs 71.8±0.18µs -0.83%
Pmtree:delete 71.6±0.06µs 71.2±0.04µs -0.56%

@github-actions
Copy link

Benchmark for 05e25b7

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1323.2±40.86ns 1375.9±111.90ns +3.98%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.4±0.10µs 3.4±0.11µs 0.00%
FullMerkleTree::set 1327.7±50.16ns 1351.4±42.67ns +1.79%
OptimalMerkleTree::compute_root 1496.0±50.03ns 1542.9±174.09ns +3.14%
OptimalMerkleTree::delete 1521.3±78.60ns 1482.6±62.30ns -2.54%
OptimalMerkleTree::get 33.5±1.72ns 33.1±1.47ns -1.19%
OptimalMerkleTree::override_range 7.7±0.42µs 8.0±0.54µs +3.90%
OptimalMerkleTree::set 1495.9±93.79ns 1492.2±55.46ns -0.25%

@github-actions
Copy link

Benchmark for 0d064fb

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1555.9±91.59ns 1462.2±70.32ns -6.02%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.9±0.22µs 3.8±0.20µs -2.56%
FullMerkleTree::set 1526.7±69.19ns 1492.6±76.59ns -2.23%
OptimalMerkleTree::compute_root 1733.0±100.69ns 1737.5±96.18ns +0.26%
OptimalMerkleTree::delete 1758.1±57.24ns 1793.4±69.05ns +2.01%
OptimalMerkleTree::get 36.0±1.78ns 36.5±2.00ns +1.39%
OptimalMerkleTree::override_range 9.3±0.46µs 8.9±0.34µs -4.30%
OptimalMerkleTree::set 1754.8±79.16ns 1725.4±93.15ns -1.68%

@github-actions
Copy link

Benchmark for 0d064fb

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.4±0.00ns 1.6±0.00ns -33.33%
Pmtree::get 466.3±0.54ns 468.7±3.83ns +0.51%
Pmtree::override_range 173.7±6.71µs 177.1±12.30µs +1.96%
Pmtree::set 71.8±1.39µs 72.8±0.63µs +1.39%
Pmtree:delete 70.9±0.13µs 71.9±0.48µs +1.41%

@github-actions
Copy link

Benchmark for 05e25b7

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.9±0.15ns 2.1±0.12ns -27.59%
Pmtree::get 578.1±27.33ns 627.5±45.55ns +8.55%
Pmtree::override_range 259.2±26.29µs 267.9±29.90µs +3.36%
Pmtree::set 91.7±4.75µs 93.0±4.58µs +1.42%
Pmtree:delete 92.7±6.17µs 92.6±2.99µs -0.11%

@github-actions
Copy link

Benchmark for 451a004

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1449.0±38.98ns 1491.2±44.28ns +2.91%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.6±0.11µs 3.7±0.06µs +2.78%
FullMerkleTree::set 1428.8±32.74ns 1431.8±66.09ns +0.21%
OptimalMerkleTree::compute_root 1615.8±36.93ns 1573.5±62.21ns -2.62%
OptimalMerkleTree::delete 1595.8±41.13ns 1601.6±49.60ns +0.36%
OptimalMerkleTree::get 35.1±0.81ns 34.9±0.91ns -0.57%
OptimalMerkleTree::override_range 8.1±0.29µs 7.5±0.45µs -7.41%
OptimalMerkleTree::set 1603.8±33.65ns 1477.0±111.49ns -7.91%

@github-actions
Copy link

Benchmark for 451a004

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.7±0.14ns 1.8±0.09ns -33.33%
Pmtree::get 553.2±26.98ns 534.7±19.11ns -3.34%
Pmtree::override_range 239.4±15.73µs 242.1±13.25µs +1.13%
Pmtree::set 94.1±7.18µs 84.4±8.19µs -10.31%
Pmtree:delete 86.3±4.37µs 82.5±3.52µs -4.40%

@rymnc rymnc marked this pull request as ready for review July 31, 2023 09:05
@github-actions
Copy link

Benchmark for 130fa23

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1219.8±2.07ns 1211.1±1.49ns -0.71%
FullMerkleTree::get 0.1±0.00ns 0.1±0.00ns 0.00%
FullMerkleTree::override_range 3.0±0.00µs 3.1±0.00µs +3.33%
FullMerkleTree::set 1213.2±2.30ns 1230.6±1.85ns +1.43%
OptimalMerkleTree::compute_root 1386.0±1.43ns 1419.1±1.15ns +2.39%
OptimalMerkleTree::delete 1378.2±1.44ns 1412.0±1.80ns +2.45%
OptimalMerkleTree::get 30.6±0.04ns 30.6±0.06ns 0.00%
OptimalMerkleTree::override_range 7.0±0.01µs 7.1±0.01µs +1.43%
OptimalMerkleTree::set 1384.9±1.83ns 1373.2±1.39ns -0.84%

@github-actions
Copy link

Benchmark for 130fa23

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.7±0.19ns 2.5±0.12ns -7.41%
Pmtree::get 558.5±22.99ns 580.3±33.18ns +3.90%
Pmtree::override_range 244.5±18.26µs 258.6±36.78µs +5.77%
Pmtree::set 86.5±4.52µs 95.0±4.69µs +9.83%
Pmtree:delete 88.3±4.00µs 91.0±5.07µs +3.06%

@github-actions
Copy link

Benchmark for f74dbfb

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1635.8±97.80ns 1609.1±53.50ns -1.63%
FullMerkleTree::get 0.1±0.00ns 0.1±0.00ns 0.00%
FullMerkleTree::override_range 4.2±0.16µs 428.6±31.43ns -89.80%
FullMerkleTree::set 1655.3±211.87ns 1559.6±59.26ns -5.78%
OptimalMerkleTree::compute_root 1808.1±63.91ns 1798.2±70.87ns -0.55%
OptimalMerkleTree::delete 1849.4±132.43ns 1863.4±60.07ns +0.76%
OptimalMerkleTree::get 38.7±1.21ns 35.1±1.46ns -9.30%
OptimalMerkleTree::override_range 9.2±0.32µs N/A N/A
OptimalMerkleTree::set 1806.6±66.95ns 1898.4±79.25ns +5.08%
OptimalMerkleTree::set_range 437.8±20.72ns N/A N/A

Comment on lines +1529 to +1540
let mut buffer = Cursor::new(Vec::<u8>::new());
rln.get_root(&mut buffer).unwrap();
let (root_after_deletion, _) = bytes_le_to_fr(&buffer.into_inner());

assert_ne!(root_after_insertion, root_after_deletion);

// We get the leaf
let mut output_buffer = Cursor::new(Vec::<u8>::new());
rln.get_leaf(set_index, &mut output_buffer).unwrap();
let (received_leaf, _) = bytes_le_to_fr(output_buffer.into_inner().as_ref());

assert_eq!(received_leaf, Fr::from(0));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this should address the edge case?

@github-actions
Copy link

Benchmark for f06cb13

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1434.8±57.47ns 1488.4±142.38ns +3.74%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.6±0.14µs 3.6±0.30µs 0.00%
FullMerkleTree::set 1423.9±61.37ns 1441.0±58.88ns +1.20%
OptimalMerkleTree::compute_root 1587.9±69.20ns 1614.6±77.12ns +1.68%
OptimalMerkleTree::delete 1590.9±51.75ns 1642.4±51.83ns +3.24%
OptimalMerkleTree::get 34.1±1.62ns 35.6±1.50ns +4.40%
OptimalMerkleTree::override_range 8.1±0.55µs 8.6±0.42µs +6.17%
OptimalMerkleTree::set 1592.0±66.77ns 1666.0±72.47ns +4.65%

@github-actions
Copy link

Benchmark for f06cb13

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.3±0.00ns 1.7±0.00ns -26.09%
Pmtree::get 458.6±0.24ns 459.4±0.18ns +0.17%
Pmtree::override_range 179.1±6.69µs 181.4±19.18µs +1.28%
Pmtree::set 71.4±0.64µs 68.8±0.04µs -3.64%
Pmtree:delete 70.8±0.05µs 69.6±0.04µs -1.69%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for 5f01a8d

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1254.7±1.58ns 1247.9±1.19ns -0.54%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.1±0.00µs 3.2±0.01µs +3.23%
FullMerkleTree::set 1250.3±1.03ns 1256.6±1.34ns +0.50%
OptimalMerkleTree::compute_root 1390.5±1.13ns 1390.9±1.42ns +0.03%
OptimalMerkleTree::delete 1388.3±0.92ns 1385.6±1.05ns -0.19%
OptimalMerkleTree::get 33.5±0.03ns 29.8±0.03ns -11.04%
OptimalMerkleTree::override_range 7.0±0.01µs 7.0±0.05µs 0.00%
OptimalMerkleTree::set 1393.0±0.83ns 1391.1±9.30ns -0.14%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for 5f01a8d

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.3±0.00ns 9.0±0.03ns +291.30%
Pmtree::get 472.8±1.43ns 472.0±0.58ns -0.17%
Pmtree::override_range 176.8±6.81µs 184.7±24.29µs +4.47%
Pmtree::set 70.7±0.07µs 70.0±0.90µs -0.99%
Pmtree:delete 70.9±0.06µs 69.4±0.06µs -2.12%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for b44274d

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1493.8±3.91ns 1519.2±10.16ns +1.70%
FullMerkleTree::get 0.1±0.00ns 0.1±0.00ns 0.00%
FullMerkleTree::override_range 3.8±0.02µs 3.8±0.01µs 0.00%
FullMerkleTree::set 1507.7±4.29ns 1521.4±10.20ns +0.91%
OptimalMerkleTree::compute_root 1670.2±5.84ns 1673.0±7.92ns +0.17%
OptimalMerkleTree::delete 1667.1±5.93ns 1663.1±11.14ns -0.24%
OptimalMerkleTree::get 34.9±0.18ns 35.8±0.19ns +2.58%
OptimalMerkleTree::override_range 8.4±0.02µs 8.7±0.05µs +3.57%
OptimalMerkleTree::set 1668.0±12.53ns 1665.0±15.73ns -0.18%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for b44274d

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.8±0.03ns 2.0±0.02ns -28.57%
Pmtree::get 547.9±6.47ns 551.2±6.28ns +0.60%
Pmtree::override_range 205.0±6.61µs 205.9±7.73µs +0.44%
Pmtree::set 83.4±0.97µs 84.4±0.91µs +1.20%
Pmtree:delete 83.6±0.99µs 83.9±0.89µs +0.36%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for a11da1a

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1264.2±1.03ns 1239.4±0.88ns -1.96%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.1±0.00µs 3.1±0.00µs 0.00%
FullMerkleTree::set 1248.6±1.22ns 1246.6±7.19ns -0.16%
OptimalMerkleTree::compute_root 1387.5±1.10ns 1395.4±1.46ns +0.57%
OptimalMerkleTree::delete 1384.6±0.77ns 1392.3±1.38ns +0.56%
OptimalMerkleTree::get 29.4±0.33ns 28.9±0.10ns -1.70%
OptimalMerkleTree::override_range 7.0±0.01µs 7.1±0.07µs +1.43%
OptimalMerkleTree::set 1390.7±14.94ns 1391.0±12.68ns +0.02%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for a11da1a

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.9±0.14ns 2.0±0.07ns -31.03%
Pmtree::get 556.2±0.62ns 578.7±19.73ns +4.05%
Pmtree::override_range 220.8±28.20µs 212.9±10.98µs -3.58%
Pmtree::set 85.3±0.08µs 85.3±1.92µs 0.00%
Pmtree:delete 85.0±0.10µs 85.2±0.06µs +0.24%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for 60a8f16

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1211.3±1.62ns 1213.1±2.84ns +0.15%
FullMerkleTree::get 0.1±0.00ns 0.1±0.00ns 0.00%
FullMerkleTree::override_range 3.1±0.00µs 3.1±0.00µs 0.00%
FullMerkleTree::set 1225.3±2.54ns 1209.8±1.38ns -1.26%
OptimalMerkleTree::compute_root 1388.4±1.51ns 1395.5±2.22ns +0.51%
OptimalMerkleTree::delete 1384.1±1.43ns 1397.9±1.82ns +1.00%
OptimalMerkleTree::get 30.6±0.06ns 30.6±0.08ns 0.00%
OptimalMerkleTree::override_range 7.0±0.01µs 7.1±0.01µs +1.43%
OptimalMerkleTree::set 1379.7±1.87ns 1386.0±2.02ns +0.46%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for 60a8f16

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.8±0.06ns 2.4±0.04ns -14.29%
Pmtree::get 559.2±5.31ns 549.5±12.55ns -1.73%
Pmtree::override_range 205.9±8.21µs 207.7±13.35µs +0.87%
Pmtree::set 84.4±3.06µs 80.3±2.32µs -4.86%
Pmtree:delete 84.6±0.61µs 80.0±1.90µs -5.44%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for cc50d40

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1595.3±73.64ns 1586.8±262.25ns -0.53%
FullMerkleTree::get 0.1±0.01ns 0.1±0.00ns 0.00%
FullMerkleTree::override_range 4.0±0.19µs 4.0±0.35µs 0.00%
FullMerkleTree::set 1605.2±83.35ns 1572.0±98.59ns -2.07%
OptimalMerkleTree::compute_root 1855.8±131.07ns 1805.7±154.69ns -2.70%
OptimalMerkleTree::delete 1771.1±82.36ns 1749.6±88.01ns -1.21%
OptimalMerkleTree::get 37.6±1.78ns 37.9±2.19ns +0.80%
OptimalMerkleTree::override_range 9.1±0.57µs 9.2±0.63µs +1.10%
OptimalMerkleTree::set 1765.8±90.25ns 1737.5±85.91ns -1.60%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for cc50d40

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.3±0.00ns 1.7±0.00ns -26.09%
Pmtree::get 468.0±0.24ns 465.6±3.59ns -0.51%
Pmtree::override_range 181.8±10.78µs 179.3±12.70µs -1.38%
Pmtree::set 71.3±0.06µs 68.8±0.07µs -3.51%
Pmtree:delete 71.6±0.07µs 68.8±0.22µs -3.91%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for a91dfe5

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1244.3±1.47ns 1239.5±0.58ns -0.39%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.1±0.01µs 3.2±0.00µs +3.23%
FullMerkleTree::set 1255.1±1.22ns 1256.8±3.02ns +0.14%
OptimalMerkleTree::compute_root 1393.1±1.19ns 1396.2±1.64ns +0.22%
OptimalMerkleTree::delete 1386.2±1.72ns 1394.4±1.88ns +0.59%
OptimalMerkleTree::get 29.8±0.02ns 29.2±0.02ns -2.01%
OptimalMerkleTree::override_range 7.0±0.01µs 7.0±0.01µs 0.00%
OptimalMerkleTree::set 1391.1±14.09ns 1392.0±7.07ns +0.06%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for a91dfe5

Click to view benchmark
Test Base PR %
Pmtree::compute_root 3.2±0.30ns 2.3±0.14ns -28.13%
Pmtree::get 665.8±70.23ns 670.0±51.17ns +0.63%
Pmtree::override_range 303.2±30.99µs 321.6±37.20µs +6.07%
Pmtree::set 102.3±6.55µs 103.6±11.05µs +1.27%
Pmtree:delete 105.2±11.09µs 103.1±10.77µs -2.00%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for cfe3ae1

Click to view benchmark
Test Base PR %
FullMerkleTree::delete 1244.6±1.36ns 1244.1±0.66ns -0.04%
FullMerkleTree::get 0.0±0.00ns 0.0±0.00ns NaN%
FullMerkleTree::override_range 3.2±0.00µs 3.1±0.00µs -3.13%
FullMerkleTree::set 1255.2±1.38ns 1239.7±0.70ns -1.23%
OptimalMerkleTree::compute_root 1408.3±0.82ns 1400.5±79.91ns -0.55%
OptimalMerkleTree::delete 1402.6±2.83ns 1390.6±1.41ns -0.86%
OptimalMerkleTree::get 30.6±0.01ns 28.8±0.02ns -5.88%
OptimalMerkleTree::override_range 7.1±0.01µs 7.0±0.01µs -1.41%
OptimalMerkleTree::set 1391.0±16.65ns 1392.7±15.46ns +0.12%

@github-actions
Copy link

github-actions bot commented Aug 1, 2023

Benchmark for cfe3ae1

Click to view benchmark
Test Base PR %
Pmtree::compute_root 2.7±0.08ns 2.3±0.07ns -14.81%
Pmtree::get 552.8±11.52ns 549.6±17.83ns -0.58%
Pmtree::override_range 204.1±10.49µs 200.4±8.92µs -1.81%
Pmtree::set 83.9±1.44µs 78.8±3.13µs -6.08%
Pmtree:delete 81.1±2.95µs 80.9±2.66µs -0.25%

@rymnc
Copy link
Contributor Author

rymnc commented Aug 1, 2023

Finally resolved with @richard-ramos! 🚀 thanks for all the help debugging!

Copy link
Member

@richard-ramos richard-ramos left a comment

Choose a reason for hiding this comment

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

🚀

@rymnc rymnc merged commit d1414a4 into master Aug 1, 2023
16 checks passed
@rymnc rymnc deleted the fix-atomic-operation-edge-case branch August 1, 2023 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
track:rln RLN Track - (Secure Messaging/Applied ZK), relay and applications
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants