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

Superapp cleanup #161

Open
kandrio opened this issue Jul 17, 2023 · 17 comments
Open

Superapp cleanup #161

kandrio opened this issue Jul 17, 2023 · 17 comments

Comments

@kandrio
Copy link
Contributor

kandrio commented Jul 17, 2023

After our discussion with @synctext (Tribler/tribler#7438 (comment)), I'm creating this "parent" issue to describe what needs to be done in the trustchain-superapp in order to make it lighter and more efficient.

Here's a list of issues we need to tackle:

  1. Keep only 6 sub-modules:
  2. Ensure that, each time the superapp runs, only the needed module gets executed, and there are no unnecessary modules loaded or running in the background.
  3. Make musicdao a mature prototype by:
  4. Add/fix Github actions

Personal fork: https://github.com/kandrio/trustchain-superapp

@kandrio
Copy link
Contributor Author

kandrio commented Jul 17, 2023

Today, I had a meeting with @InvictusRMC and we created a skeleton of my work for the upcoming weeks:

  1. Independence: Make the six modules that I mentioned in the previous comment completely independent (of one another and of other modules): Currently some of these modules import classes from other modules, so most probably I will need to move these class definitions into a common-code directory.
  2. Cleanup: Drop all other modules apart from these six.
  3. Testing: Use the espresso library (https://developer.android.com/training/testing/espresso) for e2e testing on musicdao. My work will be a baseline for testing for future projects that students will work on in the next years.

@synctext
Copy link
Member

Solid plans! Please focus on a Google Play Store and live demo for 5 Sep @ TUDelft campus. So ask @InvictusRMC for a 1 Sep .APK upload for review to Google or obtain your own account from him.

For market leaders such as GoldRepublic it would be great to offer "GoldEuro" coins within our Superapp. So the demo is for offline token transfer. Web-of-Trust is great concept to explain. https://github.com/Tribler/trustchain-superapp#digital-euro Future work: we need a simple understandable rule around offline double spending. We re-define it as "overspending" with negative balance tied to your passport-level digital identity. Irrefutable, you can't escape your fraud. Second, "first-to-claim" rule. When coming online again the ordering is determined of the claims on an "overspender". This is the order the system follows on repayment of accumulated debt by overspender.

@kandrio
Copy link
Contributor Author

kandrio commented Jul 19, 2023

MusicDAO Independence

MusicDAO depends from the following modules:

  1. currencyii (Luxury Communism): https://github.com/Tribler/trustchain-superapp/tree/master/currencyii
  2. gossipML: https://github.com/Tribler/trustchain-superapp/tree/master/gossipML

We plan to get rid of both of the above modules. Therefore, I will most probably put them under the common module (https://github.com/Tribler/trustchain-superapp/tree/master/common).

IDelft (valuetransfer) Independence

IDelft depends from the following modules:

  1. peerchat: https://github.com/Tribler/trustchain-superapp/tree/master/peerchat
  2. ig-ssi (18+): https://github.com/Tribler/trustchain-superapp/tree/master/ig-ssi

@kandrio
Copy link
Contributor Author

kandrio commented Jul 21, 2023

Pruning of old, unused modules

Personal Fork

I've successfully pruned the following modules in my personal fork (https://github.com/kandrio/trustchain-superapp):

  • DeToks
  • LiteratureDAO
  • AtomicSwap
  • TrustchainPayloadGenerator
  • TrustchainTrader
  • TrustchainVoter
  • TrustchainExplorer
  • LiquidityPool
  • Distributed AI

Upstream

I've successfully pruned from our GitHub repo the following modules:

  • DeToks
  • LiteratureDAO
  • AtomicSwap
  • TrustchainPayloadGenerator
  • TrustchainTrader
  • TrustchainExplorer
  • TrustchainVoter
  • LiquidityPool
  • Distributed AI

Relevant PR: #162

Pruning of modules that are dependencies

In this section we list modules that are harder to prune because they are dependencies for the modules that we plan to keep.

Personal Fork

I've successfully pruned the following modules in my personal fork (https://github.com/kandrio/trustchain-superapp):

  • currencyii
  • gossipML
  • peerchat
  • ig-ssi

Upstream

I've successfully pruned from our GitHub repo the following modules:

@kandrio
Copy link
Contributor Author

kandrio commented Jul 24, 2023

Trustchain Explorer and Notification

As specified in the comment above (#161 (comment)), the Trustchain Explorer is one of the modules we need to get rid of. However, there is one issue: the parent app invokes the TrustChainExplorerActivity Activity whenever the user clicks on the notification. Here's a more detailed explanation of the issue...

Issue with Trustchain Explorer

When the user installs the Trustchain Superapp on their device, there is a notification that looks like this:

When clicking on that notification, the Trustchain Explorer module opens up:

This is expected because in the TrustChainService.createNotification() member function, we set that the TrustChainExplorerActivity will get executed when the user taps on the notification. Here's the corresponding line of code:

Decisions to make

  1. Given the new information I found above, should we get rid of the Trustchain Explorer completely, or should we keep it? Maybe we want it to get spawned whenever the user taps on the notification, maybe not...
  2. If get rid of the Trustchain Explorer, what Activity will the notification spawn instead?

My personal opinion: We should get rid of the notification entirely. It does not seem to add to the user experience (constantly present on the drop-bar even if the user taps on it) and it does not provide much useful information (statically showing: "Running"). Followingly, we get rid of the Trustchain Explorer as well.

UPDATE: Feasible Solution

It turns out that we can't easily get rid of the notification. This is because the IPv8Service creates the notification for our trustchain-superapp. So, if we want to get rid of the notification, we need to update the kotlin-ipv8 code, which is in another repository. Specifically, here is the specific line where the IPv8Service creates a notification:

Therefore, one alternative solution would be to:

  • Keep the notification for now
  • Redirect the user in the central dashboard of the trustchain-superapp whenever they tap on the notification. We could easily do that by replacing the TrustChainExplorerActivity with the DashboardActivity in the following line:
    val trustChainExplorerIntent = Intent(this, TrustChainExplorerActivity::class.java)
    . This way, we can safely prune the Trustchain Explorer

CC @InvictusRMC @synctext

@synctext
Copy link
Member

synctext commented Jul 26, 2023

agreed! just get rid of this notification entirely.

@kandrio
Copy link
Contributor Author

kandrio commented Jul 26, 2023

@synctext I looked into dropping the notification entirely, but I found that we can't do it through the trustchain-superapp. Take a look at the UPDATE: Feasible Solution section of the above comment.

@synctext
Copy link
Member

good idea. Smart, keep IPv8 unpatched. please also modify the notification into something sensible, like, superapp running.

@kandrio
Copy link
Contributor Author

kandrio commented Jul 26, 2023

@InvictusRMC I created a PR (#162) where I prune all the old modules. These are the modules that are completely useless to the ones that we plan to keep. It will help to get rid of these first, and then to move to the remaining modules that need to be pruned.

The five modules that we plan to keep still depend on the following remaining modules: currencyii, peerchat, ig-ssi. I won't prune these three in this PR. However, I will get rid of them in my next PR.

@kandrio
Copy link
Contributor Author

kandrio commented Aug 3, 2023

@InvictusRMC After the above PR (#162) that prunes 9 unwanted modules, I created another PR (#163) that prunes currencyii. The latter was harder since musicdao (which we plan to keep) heavily depends on currencyii. Hence, I created a standalone PR just for the prunning of currencyii.

CC @synctext

@kandrio
Copy link
Contributor Author

kandrio commented Aug 4, 2023

@InvictusRMC I'm continuing the effort here. I created another PR (#164) that prunes ig-ssi. This one was a complex module to prune as well because iDelft depended on it.

@synctext
Copy link
Member

synctext commented Aug 4, 2023

perfect effort people!
Please plan toward a Google Playstore upload of a fully operational .apk
That process may take 7-10 days.

@kandrio
Copy link
Contributor Author

kandrio commented Aug 9, 2023

Yet another PR here:

This PR prunes the last modules in our list:

  • gossipML
  • Peerchat

Peerchat in particular was very difficult to prune as it was used not only by iDelft but also by the entire parent app.

This PR should conclude the effort of pruning all of the unnecessary modules. For a summary of what modules I pruned in total, take a look at this comment:

Summary of the cleanup work done

Here's a summary of the PRs that I've created (should be merged in that order):

  1. Prune nine old modules #162
  2. Prune the currencyii (Luxury Communism) module #163
  3. Prune the ig-ssi (18+) module #164
  4. Prune the gossipML and Peerchat modules #165

CC @InvictusRMC @synctext

@kandrio
Copy link
Contributor Author

kandrio commented Sep 9, 2023

Current State

We have:

Next Steps

Looking at the original list of tasks (#161 (comment)), I see we have the following things remaining:

  1. PeerAI is not part of Tribler/trustchain-superapp. Instead, it currently lives in @quintene's repo: https://github.com/quintene/trustchain-superapp. We should create a PR and merge that to Tribler/trustchain-superapp.
  2. Make MusicDAO a mature prototype:
    • Review and merge the recommendation tab (relevant PR: Web3Recommend #160).
    • Introduce testing (espresso library). Judging from the contents of the musicdao/src/test directory, there are no tests currently. We should start with unit tests and then add integration tests and e2e tests.
    • Fix the GUI: I don't know what that refers to. Are there specific problems with the GUI?
    • Improve code readability.
  3. Add/fix GitHub actions: I don't think that GitHub Actions are broken. Regarding adding more, any ideas?

@synctext @InvictusRMC any comments?

@InvictusRMC
Copy link
Member

InvictusRMC commented Sep 15, 2023

My ideas:

  • PeerAI: should indeed be merged. Feel free to open a PR.
  • MusicDAO: another MSc student is currently working on MusicDAO. Would let this be for now.
  • GUI tests with scenarios would be awesome!

Issues I am aware of:

  • Technical debt: tons of deprecation warnings during building. These should all be addressed at one point
  • Older APIs: we have specific logic for certain Android APIs. In some instances (e.g., permissions) this is required; in other instances, we simply perform a new action in case an older API is used. We can raise the minimum API, or fix the behavior for older APIs
  • BT: to my knowledge, currently one cannot find peers using Bluetooth unless using a very old Android version. To my understanding, the BT permissions have not been properly handled ever since Android changed it. As such, it does not function on new devices.

@InvictusRMC
Copy link
Member

Also, see this minor issue: #168

@kandrio
Copy link
Contributor Author

kandrio commented Sep 18, 2023

@InvictusRMC Thanks a lot for the update. I'm currently really busy with finishing my literature survey (Tribler/tribler#7438 (comment)). So, I'll focus on the superapp towards the end of the week.

First things I plan to kill are:

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

No branches or pull requests

3 participants