Syrius Improvements

Development ╰ Funding | Staging
103

Resolved the merge conflics of PR #21.

104

Topic moved to ╰ Funding | Staging

105

I’ll propose a code freeze for the release-v0.0.6 branch starting today, such that everyone in the community will have time to analyze the code before submitting the PRs to the official repository. Please review the commits and help me test the added features:

I’ll update this post with a CHANGELOG.md (credits to @sol, @CryptoFish and @mik3mast3rs):

Syrius v0.0.6
February 11, 2023

Features, improvements & bug fixes:

    Official support for Linux x86_64
    Updated build process to include required libraries
    Automatic Syrius releases via Github Actions
    Added Chain Identifier support in Settings -> Node Management
    Added Launch on startup support
    Added Tray manager support
    Added Desktop notifications support
    Added support for adding multiple address in Settings -> Addresses
    Added Git metadata in Info -> About
    Bridge Tab under maintenance
    Legacy swap officially deprecated
    Upgraded to Flutter 3.3.8
    Upgraded Flutter dependencies
    Updated to Material 3 theme
    Updated Community links
    Updated background image for the MacOS release
    Fixed bug related to Addresses when resetting the wallet
    Fixed bug related to scrolling inside the Info cards
    Fixed bug Dashboard -> Realtime Stats graph
    Fixed bug address reset when fusing QSR for Plasma
    Fixed bug GetIt dispose
    Fixed bug Settings -> Account-chain Stats max RPC error
1 Like
106

Zir…
image

1 Like
107

I can setup an automatic code review tool, what do you think @sol?

2 Likes
108

It could be useful but I don’t expect it to completely automate the auditing process for large PRs with multiple dependency changes.

1 Like
109

I don’t think your request to have non-technical community members randomly test your branch will yield much benefit.

We need structured testing – a checklist of actions – that can help us confirm these changes are working as expected. We should confirm these actions across all three operating systems.

As for the bugs, some of them are easily reproducible. Others are more complicated.
How do we know they’re truly fixed? Which commits fixed them?
I personally worked on some of these, so I know the details, but many community members here don’t.

For each release, someone needs to write a test plan with enough coverage to validate the items in the changelog are actually delivered. I would even accept high-level assertions like:

  • Build Syrius on Linux using an unmodified pubspec.yaml
  • Verify users can no longer swap their legacy wallets upon wallet creation
  • Verify users can create up to 10 addresses at a time

You may either break your branch/PR down into manageable pieces to simplify this process for everyone or you may write a massive, comprehensive test plan for this branch.

Please do the former.

If you’re asking me how to break it into more manageable pieces, here’s my suggestion:

  • Automatic Syrius releases via Github Actions
    Note - We need this above all else.
  • Official support for Linux x86_64
  • Updated build process to include required libraries
  • Updated background image for the MacOS release
  • Added Chain Identifier support in Settings → Node Management
  • Added Launch on startup support
  • Added Tray manager support
  • Added Desktop notifications support
  • Added support for adding multiple address in Settings → Addresses
  • Added Git metadata in Info → About
  • Bridge Tab under maintenance
  • Legacy swap officially deprecated
  • Updated Community links
  • Upgraded to Flutter 3.3.8
  • Upgraded Flutter dependencies
  • Updated to Material 3 theme
    Fixed bug related to Addresses when resetting the wallet
    Fixed bug related to scrolling inside the Info cards
    Fixed bug Dashboard -> Realtime Stats graph
    Fixed bug address reset when fusing QSR for Plasma
    Fixed bug GetIt dispose
    Fixed bug Settings -> Account-chain Stats max RPC error
4 Likes
110

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
111

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
112

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
113

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.

114

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
115

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
116

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

3 Likes
117

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

1 Like
118

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?

119

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.

120

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.

121

Okay makes sense. Thanks for clarifying.

122

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:

image
image3833×2210 268 KB

1 Like