Skip to content
This repository has been archived by the owner on Nov 15, 2021. It is now read-only.

High Volume GAS Claim Improvement #419

Merged
merged 5 commits into from
May 20, 2018

Conversation

brianlenz
Copy link
Contributor

@brianlenz brianlenz commented May 17, 2018

What current issue(s) does this address, or what feature is it adding?

Addresses with thousands of UTXOs may have issues with transaction size preventing GAS claims from working in neo-python.

@localhuman: will be curious to hear your thoughts on this. Does the approach seem reasonable? Necessary? Anything that could be better about it?

How did you solve this problem?

Claim GAS for at most 200 UTXOs at a time.

How did you make sure your solution works?

Tested and used the process.

Are there any special changes in the code that we should be aware of?

No.

Please check the following, if applicable:

  • Did you add any tests?
  • Did you run make lint?
  • Did you run make test?
  • Are you making a PR to a feature branch or development rather than master?
  • Did you add an entry to CHANGELOG.rst? (if not, please do)

* Added chunking to the GAS claim process so that at most 200 UTXOs will be claimed at a time. This allows addresses with thousands of UTXOs to claim gas through a series of transactions. A single transaction with thousands of UTXOs wasn't succeeding otherwise.
@coveralls
Copy link

coveralls commented May 17, 2018

Coverage Status

Coverage decreased (-0.003%) to 78.784% when pulling eec6e38 on brianlenz:chunk-gas-claim into 754325f on CityOfZion:development.

@localhuman
Copy link
Collaborator

Is there an example / test /use case where addresses with lots of claimable are currently failingl?

@brianlenz
Copy link
Contributor Author

brianlenz commented May 18, 2018

I've had this happen a couple of times, and the only fix I found was to chunk through the claims incrementally (which has worked without issue). The main use case for us was with our token sale SC address, which had 2k-3k claims available. At two different points in time (mid-sale and post-sale), I had tried using the standard claim process, and the tx would never go through. It would break the state of the wallet, and I'd have to rebuild the wallet. I suspected size was the issue, so I started playing around with it. I tried chunks of 500 claims at a time, and that didn't work. I found the only way around it was to chunk through the transactions in smaller increments, and 200 seemed to work in our case. It's definitely not scientific in any way, which is why I was curious to get your thoughts. If this is something that shouldn't technically be necessary, then perhaps there is some separate issue that needs to be addressed (and this approach scrapped altogether).

Unfortunately, I don't have an active example of this issue...

@localhuman
Copy link
Collaborator

Thanks for the explanation. I'd be interested in the root cause of the issue, but would have to be able to reproduce to determine that. I wonder if maybe we're running into the max size of a transaction?

Perhaps for now it would be best to allow users to pass in the size of the chunk, otherwise it defaults to current behavior?

@brianlenz
Copy link
Contributor Author

I like that idea! I'll take a crack at it :)

@brianlenz
Copy link
Contributor Author

Should be good to go now, I believe.

@localhuman
Copy link
Collaborator

Thanks for the changes!

@localhuman localhuman merged commit d78156a into CityOfZion:development May 20, 2018
@brianlenz brianlenz deleted the chunk-gas-claim branch May 20, 2018 23:41
brianlenz pushed a commit to brianlenz/neo-python that referenced this pull request May 23, 2018
metachris pushed a commit that referenced this pull request May 23, 2018
* Trivial error change for REST API when transaction is not found

* Fixing lint that I broke in #419
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants