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

fix(consensus)!: deferred execution of txs with unversioned inputs #1023

Merged

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Apr 30, 2024

Description

  • Mark transactions as deferred and execute in consensus
  • Set resolved inputs for each transaction that includes which lock is required
  • Remove input refs from transactions
  • Determine read locks from transaction execution result

Motivation and Context

Deferred execution previously had a default COMMIT decision which led to bugs when running rejected transactions (they would still get a COMMIT decision, but the validator does not have an accepted result to commit).

This PR partially implements late execution for local-only transactions. Additional work is required to support this and it has been disabled i.e. will ABORT the transaction for transactions without versions.

There was a bug where inputs without versions were provided but filled inputs for those substates were provided, the transaction would still detect that one or more inputs do not have a version.

Input refs are unnecessary because the validator will always know which substates were written at lock time.

TODOs:

  • missing transactions needs to differentiate between transactions pending execution and transactions that are deferred.
  • correct pledging procedure for unversioned inputs
  • blocks should generate a diff which is committed on a 3-chain
  • current committed state + diffs from uncommitted blocks can be used to determine inputs for subsequent transactions

How Has This Been Tested?

Existing tests pass, a new consensus unit test was written which is ignored because it fails, requires additional work to succeed, manually (previously working transactions still work).

What process can a PR reviewer use to test or verify this change?

Submit a transaction with a missing version

Breaking Changes

  • None
  • Requires data directory to be deleted
  • Other - Please specify

@sdbondi sdbondi changed the title Consensus late execution fixes fix(consensus)!: deferred execution of txs with unversioned inputs Apr 30, 2024
@sdbondi sdbondi force-pushed the consensus-late-execution-fixes branch from 38b3138 to 960f5a3 Compare April 30, 2024 10:02
Copy link

github-actions bot commented Apr 30, 2024

Test Results (CI)

533 tests  +3   532 ✅ +2   1h 36m 53s ⏱️ - 25m 45s
 63 suites ±0     0 💤 ±0 
  2 files   ±0     1 ❌ +1 

For more details on these failures, see this check.

Results for commit b2a10fe. ± Comparison against base commit a77eda6.

♻️ This comment has been updated with latest results.

stringhandler
stringhandler previously approved these changes Apr 30, 2024
Also includes:
- Indexer: Skip autofill if no required substates are provided
- Validate ID hash on transaction submits
- Allow skipping fee estimate and providing a custom fee in wallet web UI
- Clean up tx builder methods
@stringhandler stringhandler merged commit 81d884a into tari-project:development Apr 30, 2024
10 of 11 checks passed
@sdbondi sdbondi deleted the consensus-late-execution-fixes branch May 1, 2024 03:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants