search

HelixNetwork / pendulum

Pendulum is a distributed messaging protocol that enables globally available tamper proof timestamps :hourglass_flowing_sand:
https://dev.hlx.ai
Other
10 stars 6 forks source link

Inconsistent balance when spamming 0 value in testnet #208

Closed dnck closed 4 years ago

dnck commented 4 years ago

Expected Behavior

When sending 0 value tx from API to testnet nodes, I do not expect to see an "Inconsistent balance" validation failure from the WalkValidator (line 85).

Current Behavior

Like the title, I'm seeing a huge increase in validation failures when sending 0 value tx.

Steps to Reproduce

  1. Run the script, https://pastebin.com/raw/uLct6zzd

  2. After a few minutes, observe the increase in Inconsistent balance failures, http://hlx-berlin.net/d/P34o6hKZz/testnet?orgId=1&refresh=5s&from=now-1h&to=now&fullscreen&panelId=58

Context

I think this should be easy to reproduce in a development environment, but I haven't checked.

Also, it may be related to the fact that I'm using the same address to send and receive in the script, https://pastebin.com/raw/uLct6zzd

Failure Logs

relayer1 | 10/15 11:20:38.263 [XNIO-1 task-5] DEBUG WalkValidator:85 - Validation failed: 001ffdd096699faf4e9212a807dc118ac862f3cb715050756fb70faea8dfe17c balance is not consistent
cristina-vasiu commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

dnck commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

I see, is this resolved now?

cristina-vasiu commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

I see, is this resolved now?

yes, I have done a PR for it.

oracle58 commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

I think this is the case when a tx is picked up by ledgerService before it has been detected by milestoneTracker and declared as milestone. Forthcoming virtual txs would have also resolved this issue, right?

cristina-vasiu commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions). Even if it's detected by milestoneTracke, it take's its branch transaction as a normal transaction.

I think this is the case when a tx is picked up by ledgerService before it has been detected by milestoneTracker and declared as milestone. Forthcoming virtual txs would have also resolved this issue, right? yes, virtual txs should fix this kind of issues. But I have fix it until the virtual txs implementation is done.

dnck commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

Actually, I don't get it... Why should 0 value transactions have any effect on the generateBalanceDiff?

dnck commented 4 years ago

Also, did anyone manage to reproduce the original issue - that balances are inconsistent? I am testing 0 value tx again, and now tip selection fails giving the "unexpected error while generating the balance diff" message from generateBalanceDiff? This is completely new to the dev branch.

cristina-vasiu commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

Actually, I don't get it... Why should 0 value transactions have any effect on the generateBalanceDiff?

It shouldn't, but when balanced is checked for one transaction, it will travers the tangle starting with that transaction and going back to origin, until all its ancestors are done, or some error occurs, when a milestone has been reached, it can not go further with traversing the tangle, because of this current state of the balance can not be computed and retured as a null object. https://github.com/HelixNetwork/pendulum/blob/8149a0e252fdc9160a456b30eec2f2e8b25f574e/src/main/java/net/helix/pendulum/service/ledger/impl/LedgerServiceImpl.java#L154

cristina-vasiu commented 4 years ago

Also, did anyone manage to reproduce the original issue - that balances are inconsistent? I am testing 0 value tx again, and now tip selection fails giving the "unexpected error while generating the balance diff" message from generateBalanceDiff? This is completely new to the dev branch.

I have started from inial issue, I could reproduced it locally, then added the PR fix and it seems to be fixed (I have started with a clean node with args: --validator -n "udp://localhost:4101" -p 8085 -u 4100 -t 5100

oracle58 commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

Actually, I don't get it... Why should 0 value transactions have any effect on the generateBalanceDiff?

It shouldn't, but when balanced is checked for one transaction, it will travers the tangle starting with that transaction and going back to origin, until all its ancestors are done, or some error occurs, when a milestone has been reached, it can not go further with traversing the tangle, because of this current state of the balance can not be computed and retured as a null object.

https://github.com/HelixNetwork/pendulum/blob/8149a0e252fdc9160a456b30eec2f2e8b25f574e/src/main/java/net/helix/pendulum/service/ledger/impl/LedgerServiceImpl.java#L154

Yes, thats my understanding too.

Actually, I don't get it... Why should 0 value transactions have any effect on the generateBalanceDiff?

because value does not have to be spent in order for a balance diff to be created, and if there is an error, like in our case, while traversing the tangle - it will still throw "inconsistent balanceDiff" --- which might be quite irritating.

cristina-vasiu commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

Actually, I don't get it... Why should 0 value transactions have any effect on the generateBalanceDiff?

It shouldn't, but when balanced is checked for one transaction, it will travers the tangle starting with that transaction and going back to origin, until all its ancestors are done, or some error occurs, when a milestone has been reached, it can not go further with traversing the tangle, because of this current state of the balance can not be computed and retured as a null object. https://github.com/HelixNetwork/pendulum/blob/8149a0e252fdc9160a456b30eec2f2e8b25f574e/src/main/java/net/helix/pendulum/service/ledger/impl/LedgerServiceImpl.java#L154

yeah, thats my understanding as well. But, it would be odd if you (@dnck) only sent 0-value and this still occured?

it failed because of the DAG traverse (when it tries to check the balance across the DAG), not because of the balance value.

oracle58 commented 4 years ago

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

Actually, I don't get it... Why should 0 value transactions have any effect on the generateBalanceDiff?

It shouldn't, but when balanced is checked for one transaction, it will travers the tangle starting with that transaction and going back to origin, until all its ancestors are done, or some error occurs, when a milestone has been reached, it can not go further with traversing the tangle, because of this current state of the balance can not be computed and retured as a null object. https://github.com/HelixNetwork/pendulum/blob/8149a0e252fdc9160a456b30eec2f2e8b25f574e/src/main/java/net/helix/pendulum/service/ledger/impl/LedgerServiceImpl.java#L154

yeah, thats my understanding as well. But, it would be odd if you (@dnck) only sent 0-value and this still occured?

it failed because of the DAG traverse (when it tries to check the balance across the DAG), not because of the balance value.

yeah exactly, just got that :)

oracle58 commented 4 years ago

@dnck does that make sense? I will review the PR and would then close.

dnck commented 4 years ago

@dnck does that make sense? I will review the PR and would then close.

I will trust that

In LedgerService, when tangle is traversed, milestones were not processing correctly, they are processed as a normal transaction, so the milestone branch is taken, but then for this milestone branch, there was no transaction in tangle. (because milestone transaction branch is the merkle root of its transactions).

Actually, I don't get it... Why should 0 value transactions have any effect on the generateBalanceDiff?

It shouldn't, but when balanced is checked for one transaction, it will travers the tangle starting with that transaction and going back to origin, until all its ancestors are done, or some error occurs, when a milestone has been reached, it can not go further with traversing the tangle, because of this current state of the balance can not be computed and retured as a null object.

https://github.com/HelixNetwork/pendulum/blob/8149a0e252fdc9160a456b30eec2f2e8b25f574e/src/main/java/net/helix/pendulum/service/ledger/impl/LedgerServiceImpl.java#L154

I see, so actually, the fact that I was sending 0 value tx was just a correlation. It could have occured with value tx as well. Good thing you caught it!

@oliverfn I am satisfied. I see no reason not to merge and close.