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

Suggestion: Separate GNO and Tendemint Repo #577

Closed
piux2 opened this issue Mar 9, 2023 · 5 comments
Closed

Suggestion: Separate GNO and Tendemint Repo #577

piux2 opened this issue Mar 9, 2023 · 5 comments

Comments

@piux2
Copy link
Contributor

piux2 commented Mar 9, 2023

Description: Separate GNO and Tendemint Repo

We should consider separating the GNO repo from Tendermint2 as soon as possible.

We moved Tendermint2 out from GNO repo last year, but the GNO repo does not import packages from Tendermint2.

Bottomline

the physical code separation can remind and enforce us to keep ABCI as the communication protocol between the tendermint BFT consensus engine and GNO as a Cosmos application among developers.

Issue

#546 should have been implemented in the tendermint2 repo.

More Issues

Once we move #546 to the Tendermint2 repo, we will immediately see the design choice has fundamental challenges to fulfill the requirement in #575 for the following reasons.

  • The context of the original indexer implemented on tendermint differs from what we aim to achieve for GNO. Gno Vm is a module of Cosmos SDK App, any logs and events generated from the smart contract must be included in ABCI Response.DeliverTx.Log and sent through ABCI from Cosmos SDK to Tendermint2 before we can find search event information about Gnolang smart contract in the indexer.

  • The Solidity event emission is part of distributed system observability design space focusing on Log, Trace, and Metrics. The events in solidity are contract logs and call stack tracing. In Gno, log and call tracing are implemented in VM and Gnolang as part of the Cosmos App. We have to send these through ABCI to Tendermint so that the indexer can be indexed and used outside of the chain. In that case, it introduces tremendous overhead and exposes serious attacking points. For example, anyone can deploy a smart contract that generates a large-size log and call it recursively. It could flood the Tendermint Node or even the entire network.

  • For the long term, other app chains will have difficulty adopting GNO and GVM since it relies on an event server in tendermint2

@zivkovicmilos
Copy link
Member

Keep in mind #546 is not final, in a sense that it's more of a PoC where we want to test out the idea, and is definitely subject to change based on community feedback (and really their needs in terms of tx indexing). I've tried to write it as generally as possible, so that we can zero in on what makes sense and what doesn't.

Personally, I'm also a proponent of having logic separated out into different repos, because it creates not just a separation of concern on a program level (you need to import packages, and shape logic with this limitation), but also on a mental level where you really need to think about how someone will use the package you're writing (the public API).

I'd love to hear your thoughts on what kind of immediate next steps we can take - should we move all tendermint2 related development to the separate repo?

cc @moul

@piux2
Copy link
Contributor Author

piux2 commented Mar 9, 2023

Personally, I'm also a proponent of having logic separated out into different repos, because it creates not just a separation of concern on a program level (you need to import packages, and shape logic with this limitation), but also on a mental level where you really need to think about how someone will use the package you're writing (the public API).

Totally agree.

More importantly, ABCI is not only a program API but also a service API. In production environment Gno VM (Cosmos App) and Tendermint could be deployed as separately services.

As for the next step, from my point view, we should have done it yesterday. Since we will introduce IBC module and relayer in the system, things will get more complicated.

However, I assume there must be other considerations to keep the code as a single repo to today.

This is an important decision point moving forward, we might want to get input from more people and then let repo owners to make a decision within a reasonable time frame.

@moul
Copy link
Member

moul commented Mar 9, 2023

@jaekwon and I are fans of monorepos, especially at the beginning of projects when the architecture is not yet finished.
Features like ABCI could change or even eventually be entirely removed.
Keeping a monorepo lets us work on Gno-only, T2-only, or PRs updating the two projects simultaneously.


I think you should go in your direction but keep the monorepo.

  1. Move tendermint2 code from pkgs/* to tendermint2/*.
  2. Write scripts to ease synchronization between gno-Tendermint2 and upstream-Tendermint2.
  3. Optional: use go.mod's replace directive.

I also plan to set up vanity URLs for gno and tendermint2 imports. My goal was to make the two changes at the same time.

@piux2
Copy link
Contributor Author

piux2 commented Mar 10, 2023

@moul I did not know that we are going to change ABCI or even remove it entirely. In that case, I agree we might want to wait until we have clearer picture of architecture changes to avoid unnecessary over head.

@moul
Copy link
Member

moul commented Mar 10, 2023

Tendermint2 is a fork of TendermintCore that prioritizes stability and simplicity. We're currently exploring the possibility of removing or altering ABCI, even though it offers great modularity. By removing unnecessary modularity, we could potentially achieve our primary goals. However, deciding to remove ABCI is challenging because a monolith with too much logic and without ABCI's separation of concerns could make matters worse.

Once Tendermint2 becomes stable enough, we plan to encourage others to use it as a library they can trust. However, we don't want to be constrained by the decisions made by the Tendermint2 core team and its community regarding upstream features. Similarly, we don't want to assume that Gno can make all decisions for Tendermint2. That's why we intend to keep our own fork of Tendermint2 for Gno (Gno-Tendermint2), whether it's in the form of another repo in the Gno organization or a subfolder. When we eventually split the Gno monorepo, it won't be to use upstream but to establish a separate repo that we manage ourselves.

Our top priorities are to reduce the mental load and make synchronization easier. To achieve these goals, my proposal is to split the projects while maintaining the benefits of the monorepo. This approach could be extended further to reduce the mental load, such as by creating specific CI rules for Gno or Tendermint2, revamping the Makefile, or developing Tendermint2-centric binaries. By taking these steps, we hope to streamline our workflow and reduce the cognitive burden on the team.

Edit: I will test out my suggestion in #585 and let you know when I have a satisfactory proposal.

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

3 participants