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

Update encode decode so that the output is nicely formatted #1000

Merged
merged 2 commits into from
Jan 24, 2024

Conversation

forgetso
Copy link
Member

@forgetso forgetso commented Jan 24, 2024

Usage

Put any single or multiline string as an argument.

npx tsx ./dev/scripts/src/scripts/encodeDecode.ts 0x687474703a2f2f6c6f63616c686f73742f307862376539323434393434383539656564633130323564376363636163663535666232343936363331306466343837333938393032316161653335346335636631

Output:

Receive the value transformed in various ways in a table, for ease of reading, and in JSON, for ease of copying values.

TABLE OUTPUT

┌─────────┬─────────────────────────────────────┬──────────────────────────────────────────────────────────────────────────────────────────────┐
│ (index) │                name                 │                                            value                                             │
├─────────┼─────────────────────────────────────┼──────────────────────────────────────────────────────────────────────────────────────────────┤
│    0    │                'arg''0x687474703a2f2f6c6f63616c686f73742f307862376539323434393434383539656564633130323564376363' │
│    1    │                 '''636163663535666232343936363331306466343837333938393032316161653335346335636631'       │
│    2    │              'length''168'                                             │
│    3    │           'argIsAddress''true'                                            │
│    4    │             'argIsHex''true'                                            │
│    5    │ 'Failure encoding/decoding address''FAIL - Error: Expected a valid key to convert, with length 1, 2, 4, 8, 32, 33'        │
│    6    │      'Decoding hex to string''http://localhost/0xb7e9244944859eedc1025d7cccacf55fb24966310df4873989021aae354c5cf1'     │
│    7    │      'Decoding hex to number''FAIL - Error: Number can only safely store up to 53 bits'                  │
│    8    │   'Decoding string to hex to u8a''48,120,54,56,55,52,55,52,55,48,51,97,50,102,50,102,54,99,54,102,54,51,54,49,54,99,54,56,54' │
│    9    │                 ''',102,55,51,55,52,50,102,51,48,55,56,54,50,51,55,54,53,51,57,51,50,51,52,51,52,51,57,51,52,' │
│   10    │                 '''51,52,51,56,51,53,51,57,54,53,54,53,54,52,54,51,51,49,51,48,51,50,51,53,54,52,51,55,54,51,' │
│   11    │                 '''54,51,54,51,54,49,54,51,54,54,51,53,51,53,54,54,54,50,51,50,51,52,51,57,51,54,51,54,51,51,' │
│   12    │                 '''51,49,51,48,54,52,54,54,51,52,51,56,51,55,51,51,51,57,51,56,51,57,51,48,51,50,51,49,54,49,' │
│   13    │                 '''54,49,54,53,51,51,51,53,51,52,54,51,51,53,54,51,54,54,51,49'                 │
└─────────┴─────────────────────────────────────┴──────────────────────────────────────────────────────────────────────────────────────────────┘

JSON OUTPUT

[
    {
        "name": "arg",
        "value": "0x687474703a2f2f6c6f63616c686f73742f307862376539323434393434383539656564633130323564376363636163663535666232343936363331306466343837333938393032316161653335346335636631"
    },
    {
        "name": "length",
        "value": "168"
    },
    {
        "name": "argIsAddress",
        "value": "true"
    },
    {
        "name": "argIsHex",
        "value": "true"
    },
    {
        "name": "Failure encoding/decoding address",
        "value": "FAIL - Error: Expected a valid key to convert, with length 1, 2, 4, 8, 32, 33"
    },
    {
        "name": "Decoding hex to string",
        "value": "http://localhost/0xb7e9244944859eedc1025d7cccacf55fb24966310df4873989021aae354c5cf1"
    },
    {
        "name": "Decoding hex to number",
        "value": "FAIL - Error: Number can only safely store up to 53 bits"
    },
    {
        "name": "Decoding string to hex to u8a",
        "value": "48,120,54,56,55,52,55,52,55,48,51,97,50,102,50,102,54,99,54,102,54,51,54,49,54,99,54,56,54,102,55,51,55,52,50,102,51,48,55,56,54,50,51,55,54,53,51,57,51,50,51,52,51,52,51,57,51,52,51,52,51,56,51,53,51,57,54,53,54,53,54,52,54,51,51,49,51,48,51,50,51,53,54,52,51,55,54,51,54,51,54,51,54,49,54,51,54,54,51,53,51,53,54,54,54,50,51,50,51,52,51,57,51,54,51,54,51,51,51,49,51,48,54,52,54,54,51,52,51,56,51,55,51,51,51,57,51,56,51,57,51,48,51,50,51,49,54,49,54,49,54,53,51,51,51,53,51,52,54,51,51,53,54,51,54,54,51,49"
    }
]

Copy link

Pull Request Report

Hello! I'm here to provide you with a report on the changes made in the pull request. Let's get started!

Changes

  1. Updated the copyright year to 2024 in encodeDecode.ts file.
  2. Added a function wrapItemToMultipleRows to wrap table items into multiple rows.
  3. Added a function consoleTableWithWrapping to display a console table with wrapped data.
  4. Added a function isJSON to check if a string is a valid JSON.
  5. Modified the main function to include additional output and display the results in a table format.

Suggestions

I have a few suggestions to improve the code:

  1. In the wrapItemToMultipleRows function, consider using Object.entries instead of Object.values to get both the key and value of an object.
  2. In the consoleTableWithWrapping function, consider adding a default value for the maxColWidth parameter to improve readability.
  3. In the isJSON function, consider using a more descriptive variable name instead of arg to improve code clarity.

Bugs

I found a potential bug in the code:

  1. In the main function, the isJSON function is called with the argument arg, which is a string. However, the isJSON function expects a JSON string as an argument. This could lead to unexpected behavior. Please ensure that the argument passed to the isJSON function is a valid JSON string.

Improvements

I have identified a place in the code that could be refactored for better readability:

In the main function, the code block that handles the case when argIsAddress is true can be refactored to improve readability. Here's a suggested refactoring:

if (argIsAddress) {
    try {
        const encodedAddress = encodeAddress(decodeAddress(arg, false, ss58Format), ss58Format);
        const decodedAddress = decodeAddress(encodedAddress, false, ss58Format);
        if (encodedAddress === arg) {
            const hexAddress = u8aToHex(decodedAddress);
            output.push({ name: `Hex address`, value: hexAddress });
            output.push({ name: `Address bytes`, value: decodedAddress.toString() });
        } else {
            output.push({ name: `Encoded address`, value: encodedAddress });
            output.push({ name: `Address as hex`, value: u8aToHex(decodeAddress(encodedAddress)) });
        }
    } catch (e) {
        output.push({ name: `Failure encoding/decoding address`, value: `FAIL - ${e}` });
    }
}

This refactoring separates the logic for encoding and decoding addresses, making it easier to understand and maintain.

Rating

I would rate the code as follows:

  • Readability: 8/10
  • Performance: 7/10
  • Security: 9/10

The code is generally readable and well-structured. However, there are some areas where variable names could be more descriptive. The performance is decent, but there might be room for optimization in certain parts of the code. In terms of security, the code handles address encoding and decoding, which is a critical aspect, and appears to be implemented correctly.

That's all for the report! If you have any further questions or need assistance, feel free to ask. Good luck with your pull request!

@prosoponator prosoponator merged commit a1808ec into main Jan 24, 2024
9 checks passed
@prosoponator prosoponator deleted the encode-decode branch January 24, 2024 17:09
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

Successfully merging this pull request may close these issues.

3 participants