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

Add UTXO Tracking by Account #569

Closed
wants to merge 3 commits into from
Closed

Conversation

jsolman
Copy link
Contributor

@jsolman jsolman commented Jan 21, 2019

This change keeps track of unspent UTXOs (for NEO and GAS) by account script hash. Enabling this feature is controlled by a setting that defaults to false. This change also exposes the unspent UTXOs through RPC in the same JSON format that Neoscan returns in its get_balance v1 API call.

I measured the storage overhead for this feature for MainNet on Thursday, January 3, 2019. For the version of LevelDB used, the size increase on disk with this feature enabled for MainNet was only around a 6.65% increase: 11760008 KB vs. 11027004 KB.

While this feature could be implemented in a plugin. One reason to put it in the core is that the wallets code could take advantage of it instead of needing to re-index as much when opening a wallet. This change does not yet change the Wallets code to do that (best to ensure this will be able to live in the core first before potentially wasting effort to make those changes).

A major advantage of these changes is that sending NEO or GAS for arbitrary addresses would no longer require talking to another server besides a standard RPC node.

@erikzhang
Copy link
Member

We are going to abolish all global assets in NEO 3.0, including neo and gas. So there is no need to improve this in NEO 2.x.

@jsolman
Copy link
Contributor Author

jsolman commented Jan 22, 2019

That makes sense for Neo 3.0/3.x, but if Neo 2.x is always going to have the global assets, doesn't that mean it will always need it?

Is there going to be a separate branch / for Neo 3.0? At some point the code base for Neo 2.x and Neo 3.0 will need to diverge. It seems like this feature will be wanted by many on Neo 2.x.

@vncoelho
Copy link
Member

vncoelho commented Jan 22, 2019

Great to hear that, @erikzhang. Count with us!
Who is going to open the first PR with the template for the UTXO removal? If you could push the design of a basic template, @erikzhang, we can discuss and brainstorm this transition/evolution (merkle root will probably play the role of the UTXO).
@igormcoelho has some good insights for a smooth transition.

@jsolman, I think that I will probably not use NEO 2.x once nodes of NEO 3.x are released.
But the PR sounds interesting and good, @jsolman, I think that this feature is important in an UTXO model.

@erikzhang
Copy link
Member

When the NEO 3.0 network is started, the 2.x network will be stopped and the data will be retained and migrated to 3.0.

@jsolman
Copy link
Contributor Author

jsolman commented Jan 23, 2019

@erikzhang how will that work for contracts that need NEO or GAS to be a UTXO based asset?

The contract owner will definitely have to write a new contract. If part of the data included utxo hashes it doesn’t make sense to migrate that data. There would need to be some sort of token swap, but that would be very tricky for exchange contracts where the funds were under the control of user keys due to the way the contract was written.

@shargon
Copy link
Member

shargon commented Jan 23, 2019

@jsolman All contracts must be recompiled, and re-deployed

@vncoelho
Copy link
Member

vncoelho commented Jan 23, 2019

Sounds like a good business for Red4Sec....aegauehauheauhueaea
I am kidding, @shargon.... 📦
You are a master in security! Always with eagle eyes.

@jsolman
Copy link
Contributor Author

jsolman commented Jan 24, 2019

@shargon recompiled is a bit of an understatement. Definitely significant code changes will be needed to some contracts before any recompiling.

@igormcoelho
Copy link
Contributor

igormcoelho commented Jan 24, 2019

On neo/proposals neo-project/proposals#74 regarding MerklePatricia I proposed the possibility of having global assets in account format which is compatible with existing utxo, so the migration would be smooth. I agree to abolish global assets, so I guess Nep-5 will be the only asset model, right?

@erikzhang
Copy link
Member

so I guess Nep-5 will be the only asset model, right?

NEP-5, NFT, STO, ...

@shargon
Copy link
Member

shargon commented Jan 25, 2019

@jsolman I know, but this move should be required

@saltyskip
Copy link

saltyskip commented Jan 26, 2019

Maybe UTXO will be removed in NEO 3.0, but without a time on when this happens I am strongly in support of this being merged in. Centralized UTXO API's can be notoriously unstable, and the failure of one server can lead to widespread wallet outages in the NEO ecosystem

Having this merged into core would be immensely helpful for light wallets, and overall percieved stability of the main chain, as often times people view "UTXO API server failing as NEO failing" which is not true of course

We have already started running node with these changes for some parts of O3 infrastructure, and see noticeable improvements compared to relying on centralized server

@vncoelho
Copy link
Member

vncoelho commented Mar 5, 2019

If this could be merged as a plugin it would be better for merging now, avoiding future extra work in removing it.

@jsolman
Copy link
Contributor Author

jsolman commented Mar 5, 2019

I've had plans for a bit to make it a plugin. There are a few other high priorities first. I should get to it within a few weeks I suspect.

@vncoelho
Copy link
Member

vncoelho commented Mar 5, 2019

Go for it, Jeff. I think that it is interesting to have this working as a plugin for future references.

@vncoelho
Copy link
Member

vncoelho commented Mar 8, 2019

@jsolman, could you think about a flag in each UTXO?

  • initially it should be set to false
  • then, we watch all claimtransactions and set to true all UTXO that were claimed.

I think that this would be enough for us to track claimable gas and it will currently benefit several projects.

In a reasonable thinking, UTXO banning will take some more couple of months and this is a very useful PR that you did.

@jsolman
Copy link
Contributor Author

jsolman commented Mar 8, 2019

I expect to migrate this to a plugin on Sunday. After that I can look into adding claims support.

@igormcoelho
Copy link
Contributor

+1 for plugin. No need for this in core.

@jsolman
Copy link
Contributor Author

jsolman commented Mar 11, 2019

I've started to migrate this to a plugin. It will take a little longer still.

@jsolman
Copy link
Contributor Author

jsolman commented Mar 11, 2019

Closing, since this was moved to a plugin instead: neo-project/neo-modules#52

@jsolman jsolman closed this Mar 11, 2019
@shargon shargon deleted the TrackUTXOsForAllAddresses branch March 12, 2019 15:55
Thacryba pushed a commit to simplitech/neo that referenced this pull request Feb 17, 2020
Add increment package introduction for the new website.
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.

6 participants