Syrius Improvements

ZNNAYIID · January 30, 2023, 4:12pm

I agree with Sol and Vilkris.

zyler9985 · January 30, 2023, 4:15pm

Agree, it’s an important acknowledgement that not all of our validators are technical ppl, so including screenshots and descriptions is a great way to bridge the gap between technical and non-technical ppl for optimal brainstorming of feedback.

I actually think a team of translators could be made, with the sole purpose of using their technical skills to provide clarity to all of the pillars in as objective and non biased way as possible.

cryptocheshire · January 30, 2023, 4:23pm

I agree with Sol and Vilkris

coinselor · January 30, 2023, 4:50pm

I’m also in favor of smaller chunks for PRs. It seems a lot easier for others to digest, analyze and test.

I’ve been learning flutter for a while now in order to start contributing. I also try to follow the dev threads and help testing whenever there are new releases. Looking at other open source projects, they seem to have detailed SOPs for submitting a PR, including style guides for both documentation and code.

I understand we are still in our early days of development, and we should prioritize getting code out over dedicating time into coming to a consensus on best practices for our network. However, we should not lose sight of what I consider to be an important matter. For example, I’ve seen developers raise concerns for stuff like:

coordinate the exact development environment
working off a branch controlled by a single developer
sizes of PRs

We can start panting our own “how to contribute” document with a broad brush, you all know better than me what issues are more important.

From the flutter project, they give high importance to filing an issue before every PRs, with an emphasis on writing tests and keeping the PRs as minimal as possible, isolating a single issue, so if the PR needs to get undone, you don’t lose extra work/features put into it.

Tests

Every change in the flutter/engine, flutter/flutter, flutter/plugins, and flutter/packages repos must be tested, except for PRs that:

only remove code (no modified or added lines) to remove a feature or remove dead code. (Removing code to fix a bug still needs a test.)

only affect comments (including documentation).

only affect code inside the .github directory, .cirrus.yml or .ci.yaml config files.

only affect .md files.

are generated by automated bots (rollers).

have an explicit exemption (a message from Hixie starting with test-exempt:).

My humble two cents.

0x3639 · January 30, 2023, 11:14pm

This seems like a very logical approach. I hope some of the other community contributors who are a little less technical can help with some of these recommendations, such as a “how to contribute” document.

Submitting smaller PR that can be undone w/out rolling back other valuable changes also make sense to me. I’m sure we can iterate and improve as we move forward on next versions.

aliencoder · January 31, 2023, 9:51am

Removed the KeyboardFixer patch (tested and it works without it on Flutter versions > 3.3.7
Updated the RemoveOverscrollEffect for Material 3, removed it from various places where it was unnecessary (you’ll end up with a heavy rendering tree) and enabled it globally inside MaterialApp

aliencoder · February 1, 2023, 9:55pm

New feature

Added the possibility to Save QR or Share QR for a particular address.

sol · February 2, 2023, 5:28pm

Do you have a counter-proposal? We need to come to a consensus about this soon.

aliencoder · February 3, 2023, 5:19pm

I’ll need to think and come up with a solution. How many PRs do you see for syrius and znn_sdk_dart? For go-zenon I’ve opened just 1 PR.

CryptoFish · February 3, 2023, 5:33pm

In the last 5 days I’ve created 2198 QSR transactions. Now I get the following error when looking at my account stats under the settings tab.

The AccountChainStatsBloc calls the getAccountBlocksByHeight, but an error is thrown when the account reaches a total height of bigger than 1024.

The go-zenon rpc method has the following check.

github.com

zenon-network/go-zenon/blob/c0f931d3cd9844a487ae08ed15f5c52896a9bb44/rpc/api/ledger.go#L126


      
          	if block == nil {
          		return nil, nil
          	}
          
          	return ledgerAccountBlockToRpc(l.chain, block)
          }
          func (l *LedgerApi) GetAccountBlocksByHeight(address types.Address, height, count uint64) (*AccountBlockList, error) {
          	if height == 0 {
          		return nil, ErrHeightParamIsZero
          	}
          	if count > RpcMaxCountSize {
          		return nil, ErrCountParamTooBig
          	}
          
          	accountStore := l.chain.GetFrontierAccountStore(address)
          	frontier, err := accountStore.Frontier()
          	if err != nil {
          		l.log.Error("GetAccountBlocksByHeight failed", "reason", err, "method-called", "accountStore.Frontier")
          		return nil, err
          	}
          	if frontier == nil {

The widget calls the getAccountBlocksByHeight method to show the first or last block hash and total numbers of each block type.

This could be solved by dividing the total block count by 1024 and calling the method in batches of 1024 blocks.

CryptoFish · February 3, 2023, 5:34pm

@aliencoder

I’ve set my Client chain identifier selection to 321.

I’m connected to my local devnet with Node chain identifier 321. No warning icon is shown in the node selection.

No warning is issued when changing from my local devnet node to public node with node chain identifier 1. Warning icon is show in the node selection. Chain identifier mismatch. Client chain identifier: 321, Node chain identifier: 1.

The following warning is issued when changing from the public node with node chain identifier 1 to my local devnet node with node chain identifier 321. No warning icon is shown in the node selection.

Shouldn’t the warning be issued when changing to the public node instead of the local node that matched the client chain identifier?

aliencoder · February 3, 2023, 5:57pm

Another issue is the const int rpcMaxPageSize = 1024; from the Dart SDK lib/src/client/constants.

We need to fix both @CryptoFish

aliencoder · February 3, 2023, 6:02pm

Works as intended as you client chain identifier 321 matches with the chain identifier of the node 321.

I agree that we should also check and show the warning dialog.

CryptoFish · February 3, 2023, 7:36pm

But do you not agree, that the warning message shown in the picture is not correct. I’m connecting to the local node, but the node chain identifier is 321 not 1 and therefore no warning should be shown.

aliencoder · February 3, 2023, 9:43pm

Can you please explain me again the scenario?

Possible scenarios:

Embedded node ← Web Sockets → Syrius
Local node ← Web Sockets → Syrius
Remote node ← Web Sockets + SSL → Syrius

Please note that the embedded node can have chainId != 1, it doesn’t necessarily be on mainnet. That’s why it should be mandatory to show a warning dialog regardless of the chainId of the node you’re connecting to. The embedded node is currently a safer option, but not the safest until we implement something like this.

The whole idea is to empower the user to enforce the chainId in their wallet because it is used in transaction signing.

aliencoder · February 7, 2023, 1:31pm

New improvements

Updated the chain identifier selection process
Dependency cleanup:
Updated dependencies to latest version: fl_chart, flutter_svg, flip_card, package_info_plus, device_info_plus
Replaced file_selector_platform_interface with file_selector
Removed unnecessary dependencies: network_info_plus, file_selector_windows, file_selector_windows, file_selector_macos, file_selector_linux
Added new dev dependency: dependency_validator

aliencoder · February 7, 2023, 1:35pm

We really need a good logging solution for Syrius with disk persistency.

Started implementing the recommended logging library for Dart into Syrius.

There are over 100 files where we should be able to log the errors and I bet it will very helpful for future debugging.

CryptoFish · February 8, 2023, 11:56am

Created PR #30 to fix the account stats issue.

Fix account stats max rpc error by KingGorrin · Pull Request #30 · alienc0der/syrius · GitHub

aliencoder · February 8, 2023, 5:39pm

New improvements

Implemented Dart logging
Dart format applied to the repository

aliencoder · February 9, 2023, 9:11am

New improvements

Merged @CryptoFish’s pull request