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

Expand the TransactionVerifier trait #185

Open
hackaugusto opened this issue Jan 25, 2024 · 2 comments
Open

Expand the TransactionVerifier trait #185

hackaugusto opened this issue Jan 25, 2024 · 2 comments
Milestone

Comments

@hackaugusto
Copy link
Contributor

#[async_trait]
pub trait TransactionVerifier: Send + Sync + 'static {
async fn verify_tx(
&self,
tx: SharedProvenTx,
) -> Result<(), VerifyTxError>;
}

Implementations of the above trait are responsible to validate transactions, this has to account for transactions is the store, as well as transactions currently in flight. Namely, when two transactions try to consume the same nullifier, only one can succeed.

The issue with the above traits are the following:

  • The trait doesn't expose the complete API necessary to track in-flight transactions. This needs to be done with another trait, currently ApplyBlock, to callback this object and signal that an in-flight transaction has been committed to store, allowing the object to clean its internal state. While this works, it makes it harder to follow the code, since the interface is not "self sufficient".
  • The trait doesn't have an intermediary state, between this transaction has been added to the queue, to this transaction has been added to a batch. It would be interesting to model such a state, because once fees are implemented, we would like to prioritize transactions not by arrival order, but by amount of fees paid.
@hackaugusto
Copy link
Contributor Author

hackaugusto commented Jan 25, 2024

A sketch of a possible design is (pseudo code):

#[async_trait]
pub trait TransactionPool: Send + Sync + 'static {
    async fn tx_added(&self, tx: SharedProvenTx) -> Result<(), VerifyTxError>;
    async fn batch_created(&self, tx: Vec<SharedProvenTx>) -> Result<(), VerifyTxError>;
    async fn block_commited(&self, tx: Block) -> Result<(), VerifyTxError>;
}

We may want the above as a trait because of #23, so we can implement an in-memory and DB implementations (although we sqlite we can have an in-memory DB, so that is not super necessary, and we could have a plain implementation instead).

The design above makes sense to me since the lifecycle of the transaction changes after it touches multiple components. Which is kinda of modeled in the example above.

@hackaugusto
Copy link
Contributor Author

A proposed flow is at #191 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants