Identifier: Recheck InvalidIdentifier error code in Identifier

[vatsa287]

Description

During creation of a Identifier in cord-identifier package, recheck error codes being returned even when input data length is within defined range.

Goals

• Correct error codes being returned.
• Recheck calculation of input data length.

Expected Outcome

• Creation of valid identifier within defined range, with correct error codes otherwise.

Acceptance Criteria

NA

Implementation Details

NA

Mockups / Wireframes

NA

---

Product Name

CORD

Organization Name

Dhiway

Domain

Blockchain

Tech Skills Needed

Rust

Mentor(s)

@smohan-dw @amarts

Complexity

[Medium]

Category

[Bug], [Test]

Sub Category

[Backend], [Research]

[Harsh1s]

Hello there, I investigated into this issue and found the culprit. Before I make a PR, just wanted to keep you in the loop and wanted your advice on the fix. Here's what I did and found:

Here's what I've found:

I added a debugging print statement at line 189 to print the length of the Base58 string converted from vector v: println!("Length of Base58 string: {}", v.to_base58());

Having run the test from my unmerged PR #336 with length 40 for v, The length of the Base58 string is 62, which is longer than the maximum allowed length for an identifier (MAXIMUM_IDENTIFIER_LENGTH_U32), which is 49.

The reason the Base58 string is longer is due to the nature of Base58 encoding. Base58 encoding is a type of binary-to-text encoding designed to be human-readable and avoid similar-looking characters. However, this encoding tends to increase the length of the encoded string compared to the original binary data.

The InvalidIdentifier error is thrown when the Base58 string is converted into a BoundedVec<u8, ConstU32<MAXIMUM_IDENTIFIER_LENGTH_U32>> at line 98. The BoundedVec type is a vector that has a compile-time fixed maximum length. If the length of the Vec (derived from the Base58 string) exceeds this maximum length, the try_into() call fails and returns an InvalidIdentifier error.

TLDR, We're having a couple checks for valid length, one with the input vector and one with the encoded Base58 string.

Here are the two possible solutions I propose:

1. We get rid of the Base58 string max length check and rely altogether on the existing input vec length check.
2. We create a new max length limit for Base58 encoded string that is appropriate after the encoding.

Before I make my PR, I wanted to ask what you think would be the better approach? I'll do the PR right away with your advised solution.

[vatsa287]

@Harsh1s Good find!
I would recommend to do find a maximum threshold length for a vector, so when converted to a base_58 type will always be within defined maximum_identifier_length i.e 49.

Assuming that identifier input is always based on vector, can change in identifier/curi.rs,

change to,

assuming maximum_threshold_length is found to be 31,

pub const MAXIMUM_THRESHOLD_LENGTH: usize = 31;
		ensure!(
		        // Below max check is done so final identifier is within valid length.
			(input.len() > MINIMUM_IDENTIFIER_LENGTH && input.len() < MAXIMUM_THRESHOLD_LENGTH),
			IdentifierError::InvalidIdentifierLength
		);

So with this change, the final base_58 identifier would always be within current MAXIMUM_IDENTIFIER_LENGTH.
And change error code to be InvalidIdentifierLength since it is for valid length error.

@amarts what do you say of this change, hopefully it won't break in many places?

[Harsh1s]

According to my testing the max threshold size came out to be 31. If you give the green signal, I'll make the PR right away. I'm excited to solve more issues!

[vatsa287]

@Harsh1s you can proceed with following thoughts,
In reality vector is not used as inputs to creating an identifier, it is just used in tests for simplicity. Instead it is through multiple hashes, ex. Codec::encode().

Why threshold comes out to be 31?

• When we pass a raw_space vector as input by encoding in codec, the size increase by 1.
  Say,

  // len = 31
  let raw_space = [2u8; 31].into_vec();
  
  // after Codec encoding parameter size is at 32.
  Ss58Identifier::create_identifier(&(raw_space1).encode()[..], IdentifierType::Space);
  

  So when converted to base58, it is within 49.

• When hashing is involved

  // len = 256
  let raw_space = [2u8; 256].into_vec();
  
  // always space comes out to be 32 with hashing.
  let space_digest = <Test as frame_system::Config>::Hashing::hash(&raw_space.encode()[..]);
  
  // parameter size is at 32 only irrespective of vector/string size.
  Ss58Identifier::create_identifier(&(space_digest).encode()[..], IdentifierType::Space
  
  
  

• How do I hit the error code InvalidIdentifierLength?

  Since codec always produces a hash output of 32 space, use any other hashing algorithm for final hashing, which can produce a hash of more than 32.

[rising0raj]

hey @vatsa287 I would like to work on this . I have gone through the code base can you assign this to me if it is open?

[Harsh1s]

hey @vatsa287 I would like to work on this . I have gone through the code base can you assign this to me if it is open?

Hey there, I'm already working on this (as you could see from the chat), I think it would be better if you worked on some good first issue as a start. Thanks!

[rising0raj]

hey @vatsa287 I would like to work on this . I have gone through the code base can you assign this to me if it is open?

Hey there, I'm already working on this (as you could see from the chat), I think it would be better if you worked on some good first issue as a start. Thanks!

Hey there , It is already marked closed by @vatsa287 last week , That is why i just make a request >

[VedantKhairnar]

Hey @Harsh1s @rising0raj
Are you open to contributing to the issue?
Thanks.