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

Transaction failed when multi addresses in one wallet. #1007

Closed
superboyiii opened this issue Aug 7, 2019 · 13 comments · Fixed by #1008
Closed

Transaction failed when multi addresses in one wallet. #1007

superboyiii opened this issue Aug 7, 2019 · 13 comments · Fixed by #1008
Labels
bug Used to tag confirmed bugs

Comments

@superboyiii
Copy link
Member

superboyiii commented Aug 7, 2019

It's a bit odd, but it's true. When multi addresses are generated in a wallet, no need too many, two or three addresses is enough, and then send a bit gas to each one. Then if you make a transaction and if the from address has more than double of your transfer value, the tx will be made successfully. Else, it will return Failed execution. My example is like this:
image
For example, when value is 150 or 140, it will choose the 5th or the 7th address. They have balance of 150, and 150 and 140 is more than 150/2, then execution failed.

@shargon
Copy link
Member

shargon commented Aug 7, 2019

So it fail when it need to split the amount, right?

@superboyiii
Copy link
Member Author

superboyiii commented Aug 7, 2019

Not for all, but seems for NEO transaction, 1/2 of the from address balance is a watershed. And I also find when making a GAS transaction, it'll look for the least balance(except from address) to pay for net_fee + sys_fee. If this address doesn't have enough fee, then execution fails, no matter other addresses in this wallet have enough balance or not.

@shargon
Copy link
Member

shargon commented Aug 7, 2019

I send 10 to one, 5 to other, and send back 15 to the original, and all works as expected, could you provide me a small sample ?

@nicolegys
Copy link
Contributor

@shargon here is a small sample
image

@shargon
Copy link
Member

shargon commented Aug 7, 2019

With gas could be different, because maybe it need more gas for the fee. Could you provide me a small example with neo? i think that is easier to test with neo

@superboyiii
Copy link
Member Author

superboyiii commented Aug 8, 2019

With gas could be different, because maybe it need more gas for the fee. Could you provide me a small example with neo? i think that is easier to test with neo
image
@shargon Got it? When the transfer value is more than fromAddress.balance/2, it always look for the least balance address to pay for Fee. If the gas balance in that address is not enough to pay for fee, transaction execution will fail. But if the transfer value is less than fromAddress.balance/2, transaction can be made successfully.

@shargon
Copy link
Member

shargon commented Aug 8, 2019

Step 1

image

Step 2

image

Error:
image

There is not enough gas in the wallet one for send this to wallet two

Then , in the second step of the loop

image

It fail because you don't have enough balance in the snapshot

@nicolegys
Copy link
Contributor

We change snapshot to snapshot.Clone(), then this issue is fixed.
image

@shargon
Copy link
Member

shargon commented Aug 8, 2019

This code

neo/neo/Wallets/Wallet.cs

Lines 306 to 320 in 68271a9

using (ApplicationEngine engine = ApplicationEngine.Run(script, snapshot, tx, testMode: true))
{
if (engine.State.HasFlag(VMState.FAULT))
throw new InvalidOperationException($"Failed execution for '{script.ToHexString()}'");
tx.SystemFee = Math.Max(engine.GasConsumed - ApplicationEngine.GasFree, 0);
if (tx.SystemFee > 0)
{
long d = (long)NativeContract.GAS.Factor;
long remainder = tx.SystemFee % d;
if (remainder > 0)
tx.SystemFee += d - remainder;
else if (remainder < 0)
tx.SystemFee -= remainder;
}
}

Should be executed outside the loop, and the loop, shoud be only for find the gas account

@shargon
Copy link
Member

shargon commented Aug 8, 2019

Yes, it works. Do you want to apply the patch, or i do?
image

@shargon
Copy link
Member

shargon commented Aug 8, 2019

Nice catch team 😉 ! @superboyiii @nicolegys

@nicolegys
Copy link
Contributor

You apply the patch 😉

@superboyiii
Copy link
Member Author

@shargon Thanks for your investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to tag confirmed bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants