[Core] Productionize End-to-end tx from iteration 3 demo

[Olshansk]

Description

Productionze the changes made to iteration 3 demo.

Issue

Fixes #364

Type of change

Please mark the relevant option(s):

• New feature, functionality or library
• Bug fix
• Code health or cleanup
• Major breaking change
• Documentation
• Other

List of changes

In addition to make the CLI broadcast a transaction, two major breaking changes were made

• Changed the scope of TransactionExists from the PostgresContext to the PersistenceModule
• Changed the scope of CheckTransaction from the UtilityContext to the UtilityModule

The reason for this change is because the codebase was creating unnecessary contexts when the module directly could be used. In particular, access to the mempool and the txIndexer is not scope to a context but exists in the lifecycle of the entire node. Making these changes simplified the code, end-to-end flow and is also beginning to set us up for cleaner rollbacks where contexts can be just cleared & released.

Shared Changes

• Changed the scope of TransactionExists from the PostgresContext to the PersistenceModule
• Changed the scope of CheckTransaction from the UtilityContext to the UtilityModule
• Change the bus to be a pointer receiver rather than a value receiver in all the functions it implements

RPC Changes

• Updated PostV1ClientBroadcastTxSync to broadcast the transaction it receives
• Avoid creating an unnecessary utility context and use the utility module directly

Utility Module Changes

• Introduce a general purpose HandleMessage method at the utility level
• Move the scope of CheckTransaction from the context to the module level
• Add an IsEmpty function to the Mempool
• Rename DeleteTransaction to RemoveTransaction in the mempool
• Rename LatestHeight to Height in the utilityContext
• Add comments inside CheckTransaction so its functionality is clearer
• Add comments and cleanup the code in mempool.go

Consensus Changes

• Removed unused consensus.UtilityMessage

Persistence Changes

• Changed the scope of TransactionExists from the PostgresContext to the PersistenceModule

Testing

• make develop_test
• LocalNet w/ all of the steps outlined in the README
• Followed the instructions at docs/demos/iteration_3_end_to_end_tx.md

Required Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have tested my changes using the available tooling
• I have updated the corresponding CHANGELOG

If Applicable Checklist

• I have updated the corresponding README(s); local and/or global
• I have added tests that prove my fix is effective or that my feature works
• I have added, or updated, mermaid.js diagrams in the corresponding README(s)
• I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

[Olshansk]

I've added 3 reviewers to this PR.

@jessicadaugherty - Please only focus on the documentation and instructions in docs/demos/iteration_3_end_to_end_tx.md as we will likely follow something similar for future demos.

@deblasis - Please provide feedback on overall code structure, health, cross-module interaction, etc. In particular, the changes in the scoping of some of the methods will be important for rolling back contexts in the future.

@oten91 - No need to do a deep review, but I'd appreciate your opinion on TransactionGossipMessage and how I am using this in the application-specific bus entrypoint (i.e. shared/node.go). I personally think this is beginning to set us up for a good pattern to leverage the "central nervous system" of handling events, but POV given your hands-on experience would be very valuable.

[jessicadaugherty]

Thanks @Olshansk! Lgtm.

as we will likely follow something similar for future demos

So the idea is to have a demo folder with txt guides on how to run those demos, each labeled with the demo goal (e.g. "Sending tx without fees", "Sending tx with fees", "Sending tx with errors", etc.)? Or do you imagine this same demo doc just being updated to indicate different paths/steps?

[deblasis]

Solid work. Nice comments etc.

Minor things out of scope is probably gonna be around consolidating the way we access to stuff like txIndexer not here and now :)

Also, you reminded me to add a couple of test cases to tests I introduced LOL in my in-flight PRs! 🙏

[Olshansk]

@deblasis

Minor things out of scope is probably gonna be around consolidating the way we access to stuff like txIndexer not here and now :)

For sure

Also, you reminded me to add a couple of test cases to tests I introduced LOL in my in-flight PRs! 🙏

:)

---

@jessicadaugherty

So the idea is to have a demo folder with txt guides on how to run those demos,

Yea, this is what I envisioned.

We will likely be able to copy/paste some of the commands from one demo to another, but this way each of them can stand its own.

It'll be a cool way to track progress and keep the demos separate, reproducible and show progress over time.

[Olshansk]

@jessicadaugherty Please approve the PR if it looks good to you.

@deblasis I think the build keeps failing because of #380, so PTAL at that as well.

@oten91 Once the build passes, I'll merge in even if you don't have time to take a look apriori, but would still really appreciate your feedback that we can follow up on in later PRs.

[deblasis]

@Olshansk 👍 #380 is on my list, I'm gonna give it another (final 🥷🗡️) stab after my next PR

[Olshansk]

@deblasis Just a heads up that I pushed one more commit to fix an outstanding unit test: 689f271 (#370), related to #388 which I assigned to you.

I'm going to go ahead and merge since we've reviewed the rest of the code but let me know if you see a need for any follow-ups.