feat: appeals undetermined transactions #657

kstroobants · 2024-11-28T14:52:13Z

Fixes #638

What

Added appeal window to undetermined state
Changed timestamp_accepted and added appeal_undetermined in database
Activated frontend button in undetermined state and added button also to modal
Leader only has no appeal button and no appeal window
Implemented the state transitions to process the appeal
Added test for leader appeals

Why

To add more value to the user

Testing done

tests/unit/consensus/test_base.py::test_exec_undetermined_appeal

Checks

I have tested this code
I have reviewed my own PR
I have created an issue for this PR
I have set a descriptive PR title compliant with conventional commits

Reviewing tips

Run simulator
Run test_exec_undetermined_appeal
Check code changes

…inalized rollup transaction

…op when appeal failed and succeeded

…sed on formula

…ed state and add button also to modal

- Add N+2 validators, remove leader - Use latest data of transaction when in pending state and not the old one from the crawler - Write consensus data before setting status to have it updated in frontend when going to transaction info modal - Do not deploy contract when transaction was in the undetermined state

…eals-implement-sequential-appeals-fail

…8-appeals-undetermined-transactions

…sation of transaction

…undetermined-transactions

…nly check in appeal_window

… appeal button

…king on finalized state gave a small delay in showing when appeal failed and finality window was exceeded

kstroobants · 2024-12-13T13:50:20Z

@cristiam86 In the FinalizingState class we insert transactions from the transaction.consensus_data.leader_receipt.pending_transactions. Should this also happen when the status was UNDETERMINED (line 1229 in backend/consensus/base/py)? I believe not.

If you agree, I will add:
if context.transaction.status != TransactionStatus.UNDETERMINED:
# insert transactions

cristiam86 · 2024-12-18T09:49:53Z

@cristiam86 In the FinalizingState class we insert transactions from the transaction.consensus_data.leader_receipt.pending_transactions. Should this also happen when the status was UNDETERMINED (line 1229 in backend/consensus/base/py)? I believe not.

If you agree, I will add: if context.transaction.status != TransactionStatus.UNDETERMINED: # insert transactions

@kstroobants We shouldn't emit messages when the transaction is undetermined. @kirilaa could you please confirm?

kirilaa · 2024-12-18T11:28:40Z

@cristiam86 In the FinalizingState class we insert transactions from the transaction.consensus_data.leader_receipt.pending_transactions. Should this also happen when the status was UNDETERMINED (line 1229 in backend/consensus/base/py)? I believe not.
If you agree, I will add: if context.transaction.status != TransactionStatus.UNDETERMINED: # insert transactions

@kstroobants We shouldn't emit messages when the transaction is undetermined. @kirilaa could you please confirm?

true, the messages are emitted:

on acceptance
on finalization

now we had losely defined on finalization - imho we don't emit messages for finalized undetermined tx. That's from my side. Maybe I should double check with Edgars, but I am 95% we don't emit messages on finalized undetermined txs.

…ned to finalized

… queue

sonarqubecloud · 2024-12-20T10:02:19Z

Quality Gate failed

Failed conditions
46.3% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

cristiam86 · 2024-12-20T16:04:50Z

frontend/src/components/Simulator/TransactionItem.vue

@@ -55,7 +56,7 @@ const handleSetTransactionAppeal = () => {
 watch(
  () => props.transaction.status,
  (newStatus) => {
-    if (newStatus !== 'ACCEPTED') {
+    if (newStatus !== 'ACCEPTED' && newStatus !== 'UNDETERMINED') {


@kstroobants what's the reason of this check? can't we rely in the appeal property of the transaction?

cristiam86 · 2024-12-20T16:13:36Z

tests/unit/consensus/test_base.py

+    transaction = Transaction.from_dict(
+        transactions_processor.get_transaction_by_hash(transaction.hash)
+    )
+    await ConsensusAlgorithm(None, msg_handler_mock).exec_transaction(


@kstroobants this seems like a different test, shouldn't we divide them into different ones?

cristiam86

Minor comments that would be nice to solve

kstroobants added 19 commits October 22, 2024 15:24

feat: add rollup transaction db table

021f10e

fix: drop audit table, change id and nonce of rollup, only make one f…

eca17e5

…inalized rollup transaction

create rollup transaction for every validator

9184749

add function to mock in consensus test

6daee80

feat: add appeal window, accepted queue, basic test

90ea2c1

fix: old tests were stuck on accepted state, add usage of thread

bb30590

refactor consensus into states

1e1958c

feat: added appeal flow when transaction is accepted including the lo…

2708f13

…op when appeal failed and succeeded

fix: adding tests for appeals and fixing minor bugs

e3fffde

refactor: merge main and PR #573 into this branch

3d00e34

feat: add appeal_failed in db, select new validators when appealed ba…

f5c3af1

…sed on formula

docs: cleanup consensus mechanism base file

27624bb

test: checking the number of validators for different appeals

d315fd1

feat: adding appeal window to undetermined state

76b0f6d

feat: change timestamp_accepted and add appeal_undetermined in database

00e90e1

feat: undetermined to pending, activate frontend button in undetermin…

4dfe115

…ed state and add button also to modal

feat: leader only has no appeal button and no appeal window

9cc27f1

test: add test for leader appeals

f2c626e

kstroobants self-assigned this Nov 28, 2024

kstroobants linked an issue Nov 28, 2024 that may be closed by this pull request

Appeals: undetermined transactions #638

Open

6 tasks

kstroobants requested a review from cristiam86 November 29, 2024 10:52

kstroobants and others added 8 commits December 2, 2024 20:13

refactor: merge 593-appeals-add-validators-when-appealed into 604-app…

b843377

…eals-implement-sequential-appeals-fail

refactor: merge 604-appeals-implement-sequential-appeals-fail into 63…

b7daed7

…8-appeals-undetermined-transactions

refactor: merge main into 604-appeals-implement-sequential-appeals-fail

be2be7d

refactor: merging changed file permissions

afb13f6

Merge branch 'main' into 604-appeals-implement-sequential-appeals-fail

1311304

fix: appealing a write method gave a KeyError because of wrong conver…

a14ef91

…sation of transaction

docs: update transaction_processor argument description

70a85cf

fix: all appeals disagreed because of pending_transactions type

f34870e

kstroobants added 6 commits December 13, 2024 08:52

merge 604-appeals-implement-sequential-appeals-fail into 638-appeals-…

089d7af

…undetermined-transactions

refactor: undo change because of merge

722501f

fix: do not reset finality window when leader appeal failed, leader_o…

e2c9d4a

…nly check in appeal_window

fix: set value in database migration file

498e8f8

fix: add leader_only check for modal appeal button, comment out modal…

b529c8f

… appeal button

fix: do not show appeal button when finality window is finished, chec…

698c74e

…king on finalized state gave a small delay in showing when appeal failed and finality window was exceeded

kstroobants added the run-tests label Dec 13, 2024

kstroobants added 3 commits December 19, 2024 18:43

fix: we do not emit messages when the transaction goes from undetermi…

8d26d8b

…ned to finalized

refactor: merge main into 638-appeals-undetermined-transactions

a5b15db

fix: complete merge

6024107

kstroobants marked this pull request as draft December 19, 2024 18:32

fix: redirect leader appeal is processed by appeal queue, not pending…

60d280a

… queue

kstroobants marked this pull request as ready for review December 20, 2024 09:34

fix: test increase wait time to get to finalized

8357219

kstroobants mentioned this pull request Dec 20, 2024

feat: consensus appeal success should rollback all newer transactions to pending #768

Open

4 tasks

cristiam86 reviewed Dec 20, 2024

View reviewed changes

cristiam86 approved these changes Dec 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: appeals undetermined transactions #657

feat: appeals undetermined transactions #657

kstroobants commented Nov 28, 2024 •

edited

Loading

kstroobants commented Dec 13, 2024

cristiam86 commented Dec 18, 2024 •

edited

Loading

kirilaa commented Dec 18, 2024

sonarqubecloud bot commented Dec 20, 2024

cristiam86 Dec 20, 2024

cristiam86 Dec 20, 2024

cristiam86 left a comment

feat: appeals undetermined transactions #657

Are you sure you want to change the base?

feat: appeals undetermined transactions #657

Conversation

kstroobants commented Nov 28, 2024 • edited Loading

What

Why

Testing done

Checks

Reviewing tips

kstroobants commented Dec 13, 2024

cristiam86 commented Dec 18, 2024 • edited Loading

kirilaa commented Dec 18, 2024

sonarqubecloud bot commented Dec 20, 2024

Quality Gate failed

cristiam86 Dec 20, 2024

Choose a reason for hiding this comment

cristiam86 Dec 20, 2024

Choose a reason for hiding this comment

cristiam86 left a comment

Choose a reason for hiding this comment

kstroobants commented Nov 28, 2024 •

edited

Loading

cristiam86 commented Dec 18, 2024 •

edited

Loading