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 SpentCoinState.Clone #1369

Merged
merged 5 commits into from
Feb 11, 2020

Conversation

shargon
Copy link
Member

@shargon shargon commented Dec 16, 2019

Close #1368

@shargon
Copy link
Member Author

shargon commented Jan 3, 2020

@superboyiii could you test this?

@cloud8little
Copy link
Contributor

@superboyiii could you test this?

@shargon is there any user scenario for this?

@shargon
Copy link
Member Author

shargon commented Jan 3, 2020

The UT ensure that both object can be changed without change the content of the other, I think that we need a backward compatibility test.

@superboyiii
Copy link
Member

@shargon @nicolegys is focusing on test of this PR, something still needs deep investigation. She will give you final result as soon as possible.

@nicolegys
Copy link
Contributor

TestResults:
Mainnet: compatible with 2.10.3
Testnet: 7 files different with 2.10.3/master2.x

Differences_pr1369.zip

It is strange that this pr is compatible with pr 1376 both on mainnet and testnet.

@shargon shargon mentioned this pull request Jan 7, 2020
@nicolegys
Copy link
Contributor

I found out what the problem was.
My 2.10.3 testnet node didn't install RpcNep5Tracker plugin, but pr1369/pr1376 testnet node installed RpcNep5Tracker.
I have tried several times, found out that the storage files of testnet were different when with and without RpcNep5Tracker. It's the issue of RpcNep5Tracker.
I'll sync two mainnet nodes to make sure whether the same problem exists.

@shargon
Copy link
Member Author

shargon commented Feb 5, 2020

Thanks @nicolegys we can wait, no problem :)

@nicolegys
Copy link
Contributor

@shargon Thank you for your patience in waiting for the result.
The phenomenon didn't appear on mainnet.
Anyway, this is another issue. For me, the Compatibility test of pr1369/pr1376 have passed.

@superboyiii
Copy link
Member

@shargon This PR is able to merge, but seems gitcheck is bogging down.

@shargon
Copy link
Member Author

shargon commented Feb 10, 2020

I think that neo2 is with travis and was down, @erikzhang could you help us for merge this?

@erikzhang erikzhang merged commit 41250b4 into neo-project:master-2.x Feb 11, 2020
@shargon shargon deleted the fix-clone-spent-coinstate branch February 11, 2020 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants