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

[bug] Transaction revalidation order #3318

Closed
Arjentix opened this issue Mar 22, 2023 · 3 comments
Closed

[bug] Transaction revalidation order #3318

Arjentix opened this issue Mar 22, 2023 · 3 comments
Assignees
Labels
Bug Something isn't working iroha2-dev The re-implementation of a BFT hyperledger in RUST QA-confirmed This bug is reproduced and needs a fix

Comments

@Arjentix
Copy link
Contributor

By looking at core/srd/block.rs I think there is an issue with our revalidation mechanism. Need to check if I'm right or not.

Steps to test

  1. Submit some failing transaction. I.e. spend asset that user don't have or something like that
  2. Submit transaction that could make tx 1 be successful if thix tx 2 preceeded it. I.e. mint some asset amount for user

What I expect to happen

I believe that peer other than the one you have submitted these transactions will complain about tx 1 because it should fail but it won't.

So if you submit transaction to peer 1, then it will have tx 1 failed and tx 2 succeed. And all other peers should have some failure in logs, because they will have both transactions successful.

What should happen in valid Iroha 2

Nothing wrong, just every peer agrees about tx 1 being failed

@Arjentix Arjentix added Bug Something isn't working iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Mar 22, 2023
@AlexStroke AlexStroke added the QA-pending This bug is reported but is missing an MWE label Mar 22, 2023
@SamHSmith
Copy link
Contributor

You are right there is a subtle problem here. Assume the sequence Mint, Register, Mint. The first mint fails and gets put in the rejected transactions. But in validation those get validated after, so it actually passes this time. We need to revalidate transactions, in the same order as during creation.

@appetrosyan appetrosyan added QA-confirmed This bug is reproduced and needs a fix and removed QA-pending This bug is reported but is missing an MWE labels Jun 12, 2023
@SamHSmith SamHSmith self-assigned this Jun 12, 2023
@Arjentix
Copy link
Contributor Author

But in validation those get validated after, so it actually passes this time. We need to revalidate transactions, in the same order as during creation.

Yeah, that's what I've seen in the code and decided to create this issue. Have you managed to reproduce this bug?

@SamHSmith
Copy link
Contributor

Yes I reproduced it in a test and managed to fix it.

SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 17, 2023
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 19, 2023
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 19, 2023
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 19, 2023
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 20, 2023
…ctions in block

Siged-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 20, 2023
…ctions in block

Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 20, 2023
…ctions in block

Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 21, 2023
…ctions in block

Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 21, 2023
…ctions in block

Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit to SamHSmith/iroha that referenced this issue Jun 21, 2023
…ctions in block

Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
SamHSmith added a commit that referenced this issue Jun 21, 2023
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
@timofeevmd timofeevmd self-assigned this Jun 23, 2023
mversic pushed a commit that referenced this issue Oct 17, 2023
Signed-off-by: Sam H. Smith <sam.henning.smith@protonmail.com>
@alexstroke1 alexstroke1 assigned alexstroke1 and unassigned AlexStroke Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working iroha2-dev The re-implementation of a BFT hyperledger in RUST QA-confirmed This bug is reproduced and needs a fix
Projects
None yet
Development

No branches or pull requests

6 participants