frostfs-adm: Add info to error messages #1386

potyarkin · 2024-09-20T10:46:29Z

potyarkin commented

2024-09-20 10:46:29 +00:00

These error messages bubble up to human users - adding more context will help to find the cause of the issue faster.

morph init could error out due to insufficient funds in committee wallet.

Previously this would be shown as:

Error: Insufficient funds (-511) - insufficient funds
Error: script failed (FAULT state) due to an error: at instruction 94 (ASSERT): ASSERT failed

With suggested changes:

Error: committee transaction: Insufficient funds (-511) - insufficient funds
Error: transfer failed: from=committee(NZFj94ednrga3uHBrXCJL77GdAQVA3wdMS) amount=5000000000000: script failed (FAULT state) due to an error: at instruction 94 (ASSERT): ASSERT failed

New error messages are not perfect but they are better that what we have now. Possible improvements:

Show transaction fee in insufficient funds error. I have not found a way to surface that. Fees are a visible deep inside neo-go but not at frostfs-adm level.
Show token names in addition to amounts. We can access Uint160 token ids but I don't know if we have an easy way to translate these ids into human-readable strings.
Show recepients' addresses in the second error message. I didn't add that because recepients are represented by Uint160 hash and for the sender we have full 25-byte base58 encoded address. Using different address formats in a single message would be misleading.

These error messages bubble up to human users - adding more context will help to find the cause of the issue faster. `morph init` could error out due to insufficient funds in committee wallet. Previously this would be shown as: - `Error: Insufficient funds (-511) - insufficient funds` - `Error: script failed (FAULT state) due to an error: at instruction 94 (ASSERT): ASSERT failed` With suggested changes: - `Error: committee transaction: Insufficient funds (-511) - insufficient funds` - `Error: transfer failed: from=committee(NZFj94ednrga3uHBrXCJL77GdAQVA3wdMS) amount=5000000000000: script failed (FAULT state) due to an error: at instruction 94 (ASSERT): ASSERT failed` New error messages are not perfect but they are better that what we have now. Possible improvements: - Show transaction fee in `insufficient funds` error. I have not found a way to surface that. Fees are a visible [deep inside neo-go](https://github.com/nspcc-dev/neo-go/blob/e1d5ac8557d7e087158e6a766d059913a56df79f/pkg/core/mempool/mem_pool.go#L187) but not at frostfs-adm level. - Show token names in addition to amounts. We can access `Uint160` token ids but I don't know if we have an easy way to translate these ids into human-readable strings. - Show recepients' addresses in the second error message. I didn't add that because recepients are represented by `Uint160` hash and for the sender we have full 25-byte base58 encoded address. Using different address formats in a single message would be misleading.

potyarkin added the

enhancement

frostfs-adm

P3

labels 2024-09-20 10:54:26 +00:00

dstepanov-yadro requested changes 2024-09-20 13:10:38 +00:00

Dismissed

cmd/frostfs-adm/internal/modules/morph/initialize/initialize_roles.go Outdated

					
				@ -30,3 +32,3 @@

					if err := c.SendCommitteeTx(w.Bytes(), false); err != nil {

						return err

						return fmt.Errorf("committee transaction: %w", err)

dstepanov-yadro commented

2024-09-20 13:07:02 +00:00

send committee transaction - to distinguish between errors

`send committee transaction` - to distinguish between errors

potyarkin marked this conversation as resolved

cmd/frostfs-adm/internal/modules/morph/initialize/initialize_roles.go Outdated

					
				@ -35,1 +37,3 @@

					return c.AwaitTx()

					err := c.AwaitTx()

					if err != nil {

						err = fmt.Errorf("committee transaction: %w", err)