Syrius Improvements

Thank you for taking your time to make this plan. I’ve re-tested aIl my commits and I’ll need a few more days to review all the commits again and implement the best possible strategy.

PS: I’ll also check how we can implement automatic testing into the codebase.

3 Likes

Can you please start by submitting the github actions on their own?

We only need two PRs at this time:

  • Github Actions
  • Bridge closure

Everything else is nice-to-have for v0.0.6.
I know we have a lot of content we want to submit for the next update, but let’s just focus on a small, manageable objective.

4 Likes

Hey. I’ll need to update the GA across all repos to reflect the latest changes from the Syrius cicd pipeline and start submitting the PRs.

1 Like

Regarding the codebase, we already work on separate branches as follows:

100% tested, otherwise the cicd pipeline would fail.

https://github.com/alienc0der/syrius/tree/add-multi-address

The remaining bug fixes and features are present in the dev branch. It’s harder to create unit-tests at the moment and the time is not in our favor.

Me and Mike tested the wallet thoroughly in the last 3 weeks and everything is working as expected on Windows and MacOS. @0x3639 covered the Linux testing and not a single issue was found until now.

I can implement a stacked PR strategy, but it’s time consuming and we already tested all the features manually.

The community seems to favor this approach. I know it will take more time, but we want to have as much feedback as possible.

This means making the proposed changes visible/accessible to everyone and waiting a period of time before before merging them into the official repo.

You can say “the code has been publicly available on my git repo for X months, it’s time to merge” but I think we should at least have a poll to confirm you have the community’s support.

Are you satisfied with the quality of all proposed changes in this post? https://forum2.zenon.org/t/syrius-improvements/578/113
  • Yes, submit them all in one PR
  • Yes, submit them all in separate PRs
  • No, needs more testing and/or community feedback
  • Some Yes, some No (please elaborate in a comment)
  • I don’t understand what’s being proposed here
0 voters

Some of my merged PR’s in @aliencoder cicd branch have been remade and based on the main repo master branch to further isolate them and remove any unnecessary dependencies. The original PR’s have been renamed to -archive. It doesn’t win the colleboration of the year award and I hope we’ll improve on this for upcoming changes.

9 of the 13 PR’s were isolated, the remaining 4 PR’s have a hard dependency on the cicd branch. These are the following:

The other 9 PRs have been submitted to the main repo.

4 Likes

Thank you so much @CryptoFish! I will submit the PR and hopefully we can move forward.

3 Likes

Deleted marked and merged PR’s. 2 PR’s remain open.

1 Like

Hey @aliencoder,
Two questions about the 0.0.6 update:

What’s the purpose of this change?

Why was the legacy pillar registration functionality removed?

Because the znn_swap_utility was incompatible with the updated ffi package and since we don’t have the source code of the library, it was impossible to build an arm64 version.

Also I saw that you’ve provided a tool to register legacy Pillars.

When you clone Syrius from Github, the directory where the source code resides is called syrius.

An important Syrius improvement that should be implemented is to persist the Logger messages in a file, similar to what go-zenon does in the .znn/log/ folder.

For example @0x3639 encountered a strange situation where a user was unable to unlock the wallet after importing the seed.

Okay makes sense. Thanks for clarifying.

Does anyone oppose changing the copy button to not show the basic notification but rather a toast message and an animated checkmark when clicked? This would also mean that the copy notification actions wouldn’t be stored into the DB anymore, but I don’t really see much value in that anyway.

I posted a demo video in the Discord dev chat since Discourse doesn’t support videos.

This change would be included with the P2P swap feature since the motivation for the change came from the swap modals.

In this image Address 1 has just been copied:

1 Like

Feedback: you can customize a little bit the toast: green color, add a suggestive icon maybe

1 Like

Regarding Syrius v0.0.7 release:

Hopefully we can integrate my WalletConnect implementation before the bridge goes live next week.

Why?

Because most of you I think are more comfortable using the Syrius desktop wallet rather than the browser extension.

4 Likes

Certainly, since most users already have the mnemonic imported in syrius.

2 Likes

100% agree on this.

Love the checks!

Will there be a address book in 0.0.7?
Option to name accounts?

Can be easily implemented, but it’s not a priority right now.

You already have this option right now.

1 Like