neo-project
[Neo Core Bug] fix compound type reference issue
Description
This one serves as similier purpose of https://github.com/neo-project/neo/pull/3325. This pr addresses the issue of compound type can add items that contain subitems not added to the referenc counter.
This pr ensures that all items be correctly calculated to the reference counter.
Fixes # (issue)
Type of change
- [ ] Optimization (the change is only an optimization)
- [ ] Style (the change is only a code style for better maintenance or standard purpose)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] This change requires a documentation update
How Has This Been Tested?
- [x] TestInvalidReferenceStackItem
Test Configuration:
Checklist:
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my code
- [x] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my feature works
- [x] New and existing unit tests pass locally with my changes
- [x] Any dependent changes have been merged and published in downstream modules
Couls you explain this solution?
Deepcopy can rebuild the reference relationship, add those not calculated items into the reference counter. So, I will check every single compound type that is added to the array, if that compound has no referecencounter, means its not calculated with reference counter, so i assign it the reference counter and deepcopy it, such that we not only add the compound itself, but also add every single subitem in that compound type.
For instance:
arr = Array(referencecounter);
arr2 = Array(){1,2,3,4,5,6,7};
arr.add(arr2); \ previously this will only add one reference counter, all subitems in the arr2 will not be calculated.
But with this pr, arr will know arr2 is a compound type and its referencecounter is null, so it will not add arr2, but add a deepcopied arr2, which has all its subitems calculated with referencecounter.
But I think that the bug can be replicated without decopy 🤔
There will always be another solution, just depends on how you want it to be solved. The problem is does this pr fix the issue.
BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3880/23H2/2023Update/SunValley3) Intel Core i9-14900HX, 1 CPU, 32 logical and 24 physical cores .NET SDK 8.0.205 [Host] : .NET 8.0.5 (8.0.524.21615), X64 RyuJIT AVX2 DefaultJob : .NET 8.0.5 (8.0.524.21615), X64 RyuJIT AVX2
| Method | Params | Mean | Error | StdDev | Median |
|---|---|---|---|---|---|
| BenchNestedArrayDeepCopy | (1, 1) | 342.7 ns | 7.33 ns | 21.50 ns | 340.5 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (1, 1) | 288.8 ns | 5.49 ns | 12.28 ns | 285.9 ns |
| BenchNestedTestArrayDeepCopy | (1, 1) | 243.8 ns | 2.89 ns | 2.70 ns | 242.8 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (1, 1) | 286.3 ns | 2.35 ns | 2.20 ns | 285.8 ns |
| BenchNestedArrayDeepCopy | (1, 2) | 498.8 ns | 5.65 ns | 5.29 ns | 496.3 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (1, 2) | 415.6 ns | 3.78 ns | 3.54 ns | 415.0 ns |
| BenchNestedTestArrayDeepCopy | (1, 2) | 363.9 ns | 2.83 ns | 2.65 ns | 364.1 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (1, 2) | 413.0 ns | 2.89 ns | 2.56 ns | 412.5 ns |
| BenchNestedArrayDeepCopy | (1, 4) | 942.6 ns | 7.98 ns | 7.47 ns | 943.2 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (1, 4) | 785.1 ns | 5.94 ns | 5.27 ns | 783.9 ns |
| BenchNestedTestArrayDeepCopy | (1, 4) | 662.3 ns | 4.96 ns | 4.64 ns | 661.1 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (1, 4) | 785.8 ns | 7.59 ns | 7.10 ns | 783.8 ns |
| BenchNestedArrayDeepCopy | (1, 8) | 1,938.0 ns | 22.97 ns | 21.49 ns | 1,935.1 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (1, 8) | 1,553.8 ns | 14.70 ns | 13.75 ns | 1,550.5 ns |
| BenchNestedTestArrayDeepCopy | (1, 8) | 1,317.0 ns | 14.75 ns | 13.08 ns | 1,317.5 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (1, 8) | 1,540.5 ns | 16.88 ns | 14.97 ns | 1,538.9 ns |
| BenchNestedArrayDeepCopy | (2, 1) | 519.1 ns | 6.16 ns | 5.77 ns | 517.7 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (2, 1) | 426.0 ns | 2.52 ns | 2.35 ns | 424.7 ns |
| BenchNestedTestArrayDeepCopy | (2, 1) | 299.2 ns | 2.41 ns | 2.01 ns | 298.7 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (2, 1) | 433.8 ns | 5.08 ns | 4.75 ns | 432.7 ns |
| BenchNestedArrayDeepCopy | (2, 2) | 1,364.1 ns | 12.05 ns | 11.27 ns | 1,359.2 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (2, 2) | 1,120.2 ns | 7.79 ns | 6.08 ns | 1,118.4 ns |
| BenchNestedTestArrayDeepCopy | (2, 2) | 601.1 ns | 6.54 ns | 5.80 ns | 599.2 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (2, 2) | 1,105.8 ns | 12.09 ns | 11.31 ns | 1,104.7 ns |
| BenchNestedArrayDeepCopy | (2, 4) | 4,339.6 ns | 37.40 ns | 33.16 ns | 4,337.2 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (2, 4) | 3,559.7 ns | 23.42 ns | 21.90 ns | 3,561.1 ns |
| BenchNestedTestArrayDeepCopy | (2, 4) | 1,578.1 ns | 13.08 ns | 12.23 ns | 1,581.3 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (2, 4) | 3,629.1 ns | 60.46 ns | 71.98 ns | 3,611.7 ns |
| BenchNestedArrayDeepCopy | (2, 8) | 15,161.5 ns | 284.68 ns | 279.60 ns | 15,134.7 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (2, 8) | 12,655.0 ns | 251.70 ns | 352.85 ns | 12,746.1 ns |
| BenchNestedTestArrayDeepCopy | (2, 8) | 4,876.1 ns | 96.47 ns | 176.39 ns | 4,817.0 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (2, 8) | 11,934.8 ns | 228.58 ns | 224.50 ns | 11,924.4 ns |
| BenchNestedArrayDeepCopy | (4, 1) | 1,002.1 ns | 17.35 ns | 16.23 ns | 1,005.1 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (4, 1) | 837.7 ns | 14.06 ns | 13.15 ns | 833.4 ns |
| BenchNestedTestArrayDeepCopy | (4, 1) | 433.5 ns | 5.17 ns | 4.04 ns | 432.4 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (4, 1) | 832.6 ns | 6.60 ns | 6.17 ns | 832.1 ns |
| BenchNestedArrayDeepCopy | (4, 2) | 6,382.8 ns | 52.77 ns | 44.06 ns | 6,369.0 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (4, 2) | 5,198.7 ns | 103.92 ns | 111.19 ns | 5,212.9 ns |
| BenchNestedTestArrayDeepCopy | (4, 2) | 1,969.3 ns | 33.25 ns | 31.10 ns | 1,972.6 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (4, 2) | 5,128.0 ns | 52.40 ns | 49.01 ns | 5,097.1 ns |
| BenchNestedArrayDeepCopy | (4, 4) | 77,014.5 ns | 1,536.10 ns | 4,100.16 ns | 75,942.0 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (4, 4) | 60,333.2 ns | 1,170.70 ns | 1,437.73 ns | 60,137.2 ns |
| BenchNestedTestArrayDeepCopy | (4, 4) | 17,854.7 ns | 322.02 ns | 285.46 ns | 17,743.9 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (4, 4) | 58,003.2 ns | 981.22 ns | 819.36 ns | 57,759.1 ns |
| BenchNestedArrayDeepCopy | (4, 8) | 2,199,376.3 ns | 41,871.98 ns | 67,615.45 ns | 2,215,854.7 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (4, 8) | 1,695,943.3 ns | 32,124.96 ns | 62,657.26 ns | 1,694,366.6 ns |
| BenchNestedTestArrayDeepCopy | (4, 8) | 438,148.5 ns | 8,662.74 ns | 15,840.31 ns | 437,996.0 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (4, 8) | 1,609,254.3 ns | 30,799.54 ns | 32,955.16 ns | 1,602,670.7 ns |
| BenchNestedArrayDeepCopy | (8, 1) | 1,927.7 ns | 38.27 ns | 78.18 ns | 1,913.7 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (8, 1) | 1,583.2 ns | 11.60 ns | 10.85 ns | 1,582.5 ns |
| BenchNestedTestArrayDeepCopy | (8, 1) | 683.8 ns | 5.48 ns | 5.13 ns | 682.7 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (8, 1) | 1,589.8 ns | 13.69 ns | 12.81 ns | 1,594.5 ns |
| BenchNestedArrayDeepCopy | (8, 2) | 118,258.4 ns | 982.00 ns | 918.56 ns | 118,253.0 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (8, 2) | 94,085.7 ns | 930.96 ns | 870.82 ns | 94,000.1 ns |
| BenchNestedTestArrayDeepCopy | (8, 2) | 32,212.5 ns | 611.43 ns | 600.51 ns | 31,878.1 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (8, 2) | 93,682.6 ns | 900.93 ns | 842.73 ns | 93,619.1 ns |
| BenchNestedArrayDeepCopy | (8, 4) | 169,669,360.4 ns | 3,355,690.79 ns | 9,014,845.40 ns | 171,844,412.5 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (8, 4) | 122,859,534.5 ns | 2,418,377.86 ns | 4,422,140.67 ns | 123,617,262.5 ns |
| BenchNestedTestArrayDeepCopy | (8, 4) | 33,864,065.4 ns | 655,980.72 ns | 940,787.95 ns | 33,728,600.0 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (8, 4) | 122,422,048.6 ns | 2,429,404.01 ns | 6,004,884.81 ns | 123,007,820.0 ns |
| BenchNestedArrayDeepCopy | (8, 8) | 40,992,899,973.3 ns | 438,805,673.14 ns | 410,459,098.91 ns | 40,981,608,100.0 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (8, 8) | 32,127,834,113.3 ns | 508,499,401.55 ns | 475,650,655.71 ns | 32,224,817,100.0 ns |
| BenchNestedTestArrayDeepCopy | (8, 8) | 10,234,584,550.0 ns | 202,267,927.80 ns | 348,902,237.42 ns | 10,279,612,600.0 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (8, 8) | 33,541,280,817.4 ns | 643,685,928.91 ns | 814,056,600.31 ns | 33,609,456,200.0 ns |
| BenchNestedArrayDeepCopy | (16, 1) | 3,552.6 ns | 69.79 ns | 163.13 ns | 3,516.8 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (16, 1) | 2,912.8 ns | 53.35 ns | 134.83 ns | 2,852.2 ns |
| BenchNestedTestArrayDeepCopy | (16, 1) | 1,240.0 ns | 32.92 ns | 93.91 ns | 1,213.0 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (16, 1) | 3,517.9 ns | 78.82 ns | 232.41 ns | 3,441.4 ns |
| BenchNestedArrayDeepCopy | (16, 2) | 257,862,647.2 ns | 5,155,211.45 ns | 8,613,200.24 ns | 257,611,450.0 ns |
| BenchNestedArrayDeepCopyWithReferenceCounter | (16, 2) | 187,807,686.8 ns | 3,729,102.21 ns | 8,185,469.76 ns | 188,697,883.3 ns |
| BenchNestedTestArrayDeepCopy | (16, 2) | 51,730,038.3 ns | 1,021,419.37 ns | 1,678,221.31 ns | 51,624,790.0 ns |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (16, 2) | 186,950,476.6 ns | 3,722,915.31 ns | 8,628,422.34 ns | 186,145,150.0 ns |
| Method | Params | Mean | Error | StdDev |
|---|---|---|---|---|
| BenchNestedArrayDeepCopy | (4, 6) | 413.90 us | 8.113 us | 11.635 us |
| BenchNestedArrayDeepCopyWithReferenceCounter | (4, 6) | 313.05 us | 6.172 us | 12.883 us |
| BenchNestedTestArrayDeepCopy | (4, 6) | 92.82 us | 0.966 us | 0.903 us |
| BenchNestedTestArrayDeepCopyWithReferenceCounter | (4, 6) | 302.10 us | 4.965 us | 4.644 us |
@roman-khimov @shargon @cschuchardt88 @vncoelho benchmark shows that rebuild the reference reelationship will incur 4 times overhead....thus i suggest to throw exception directly. User will not be able to construst an compoundtype without referencecounter from the contract, thus the only plaace that can exploit it by our misimplementing.....