Fixed hashcode for StackItem
Description
Close https://github.com/neo-project/neo/issues/3543
I don't know if this was bug in the test if someone could answer that. See below. Either way the logic for doing byte array or spans was currently incorrect for HashCode.
itemA = new VM.Types.Buffer(1);
itemB = new VM.Types.Buffer(1);
Assert.IsTrue(itemA.GetHashCode() != itemB.GetHashCode()); // this line here
Type of change
- [x] 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)
- [x] 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] Unit Tests
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
- [x] 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
Why don't return just the length?
lengths can be the same for different things. You want to treat everything your comparing to have a unique value for the same item. Hints the name HashCode
Why don't return just the length?
lengths can be the same for different things. You want to treat everything your comparing to have a unique value for the same item. Hints the name
HashCode
But it's a valid hash, when the hash is the same, Equals is called.
Why don't return just the length?
lengths can be the same for different things. You want to treat everything your comparing to have a unique value for the same item. Hints the name
HashCodeBut it's a valid hash, when the hash is the same, Equals is called.
If you implement Equals(T), you should also override the base class implementations of Equals(Object) and GetHashCode() so that their behavior is consistent with that of the Equals(T) method. If you do override Equals(Object), your overridden implementation is also called in calls to the static Equals(System.Object, System.Object) method on your class. In addition, you should overload the op_Equality and op_Inequality operators. This ensures that all tests for equality return consistent results.
Reference : https://learn.microsoft.com/en-us/dotnet/api/system.iequatable-1.equals?view=net-8.0
Also GetHashCode is used in hashtables.
Description
I don't know if this was bug in the test if someone could answer that. See below. Either way the logic for doing byte array or spans was currently incorrect for
HashCode.itemA = new VM.Types.Buffer(1); itemB = new VM.Types.Buffer(1); Assert.IsTrue(itemA.GetHashCode() != itemB.GetHashCode()); // this line hereType of change
- [x] 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)
- [x] 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] Unit Tests
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
- [x] 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
I don't see any wrong there, hashCode is a hash to reduce the calls to equal method, must be fast as possible, and should return the same value when the value could be the same.
itemA.GetHashCode() != itemB.GetHashCode() is saying there not equal, even through they are. That's not a good thing? Or is that by design?
itemA.GetHashCode() != itemB.GetHashCode()is saying there not equal, even through they are. That's not a good thing? Or is that by design?
Yes, you are right, they should return the same value for speed up
@shargon I added a simple hashing. Please check code again.
@shargon please wait, this pr can cause problem, please wait for a sec.
@cschuchardt88
My primary concern here is that this change may affect EQUAL-alike VM opcodes behaviour. From the previous discussions I see that the behaviour should be the same, but I'd insist to check this patch on the existing mainnet/testnet data and ensure that this change does not affect N3 chains states.
I have run this bench on the devpack test contracts, it behaves the same. But you are correct, @superboyiii may you please check this pr on mainnet and testnet?
My primary concern here is that this change may affect EQUAL-alike VM opcodes behaviour. From the previous discussions I see that the behaviour should be the same, but I'd insist to check this patch on the existing mainnet/testnet data and ensure that this change does not affect N3 chains states.
Equals works as before
It's also compatible on testnet. LGTM