[Neo Core Bug] fix compound type reference issue

[Jim8y]

Description

This one serves as similier purpose of #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)
• 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?

• TestInvalidReferenceStackItem

Test Configuration:

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[vncoelho]

I am in agreement with this solution, this looks like to be fully compatible.

[shargon]

Couls you explain this solution?

[Jim8y]

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.

[shargon]

But I think that the bug can be replicated without decopy 🤔

[Jim8y]

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.

[roman-khimov]

I fear this can explode, imagine deeply nested structures that you would try to deep-copy just to count references, what's gonna happen to execution time with this code?

[Jim8y]

if deepcopy can go explode, then i think it should be a problem of deepcopy, isnt it?

[AnnaShaleva]

i think it should be a problem of deepcopy

It will be a problem of the whole node.

[roman-khimov]

Think of things like #2723. DeepCopy will do its job, no doubt about it, but it will affect the cost/time balance of some opcodes. Like PICKITEM is affected by this line IIUC and it's cheap, it's supposed to be cheap.

[Jim8y]

i think it should be a problem of deepcopy

It will be a problem of the whole node.

I mean if deepcopy can explode, it means deepcopy itself is problematic, and should be fixing deepcopy, instead of here. And @roman-khimov's concern is reasonable, but if balance is a concern here, it should be a concern everywhere and everytime we make any change to the core. We dont have benchmark for them, has being working like this. But I will add a benchmark specifically for this one.

[Jim8y]

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

[Jim8y]

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

[Jim8y]

@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.....

[shargon]

LGTM but with conflicts