search

paritytech / polkadot-sdk

The Parity Polkadot Blockchain SDK
https://polkadot.network/
1.75k stars 629 forks source link

Change identity raw data values to be generic Get<u32> #3977

Open ltfschoen opened 5 months ago

ltfschoen commented 5 months ago

Is there an existing issue?

Experiencing problems? Have you tried our Stack Exchange first?

Motivation

Motivation mentioned here: https://github.com/polkadot-fellows/RFCs/pull/76/files#diff-26a34bd4a30949abd8e394e45855154a61baf508856915c9ebc105726c28e502R13

Request

Change identity raw data values to be generic Get to allow larger lengths instead of restricted to 32 bytes/chars limit.

Solution

Solution as established here https://github.com/polkadot-fellows/RFCs/pull/76

This Data type is currently re-used across both the new Rococo config and partly in Polkadot. It should be enough to make that 32 there generic as Get for the identity. Then in a second change you can increase it for the Polkadot runtime config. So first a PR to polkadot-sdk to make that value configurable and then we can propose the change in the pallet config.

Are you willing to help with this request?

Yes!

bkchr commented 5 months ago

Please go ahead with a pr @ltfschoen

ltfschoen commented 5 months ago

I can't work on this right at the moment, I'm trying to apply for the PBA "Remote Track" cohort to improve my knowledge

ggwpez commented 5 months ago

Okay thanks for letting us know. Will mark as mentor.

ltfschoen commented 5 months ago

so i've run a local node using the Polkadot SDK and exposed RPC port 9944, then if i use the current Polkadot.js Apps UI that uses Polkadot.js api v10.12.4 and do the following, I recreate the error:

So I then tried forking that branch of https://github.com/polkadot-js/api

git clone git@github.com:ltfschoen/api.git
cd api
git remote add upstream https://github.com/polkadot-js/api
git fetch upstream v10.12.4:v10.12.4
git checkout v10.12.4

Searched for the snippet of that error text Call: failed decoding throughout that codebase to find that it is triggered here https://github.com/polkadot-js/api/blob/v10.12.4/packages/types/src/generic/Call.ts#L144

But to check that my Get<u32> related identity changes to the Substrate Node will work in Polkadot.js Apps by modifying that version of the Polkadot.js API, it's necessary to first run my own local version of Poladot.js Apps, and customise the Polkadot.js API dependency that it uses so that it uses the latest metadata from my Substrate Node by following the steps described here https://github.com/polkadot-js/api/blob/v10.12.4/packages/types/src/metadata/README.md, which would be quite time consuming.

So I instead tried using SubXT https://github.com/paritytech/subxt to recreate creating that identity > setIdentity extrinsic, but I encountered errors that I didn't understand how to overcome and added TODOs in the code to highlight what i didn't understand. Here's the PR that shows the changes that I made and the error is shown in the subject of the PR https://github.com/ltfschoen/subxt/pull/1

ltfschoen commented 5 months ago

i seem to have fixed that by defining 

impl<Data> From<Vec<Data>> for BoundedVec4<Data> {
    fn from(v: Vec<Data>) -> Self {
        BoundedVec4(v)
    }
}

but i don't know why i'm using BoundedVec4 instead of BoundedVec

ltfschoen commented 5 months ago

but i was getting a timeout when connecting to Substrate node, so i tried implementing reconnecting_jsonrpsee_ws_client, but that seems to have required me to use the newtype pattern, and now i'm getting an error https://github.com/ltfschoen/subxt/pull/1#issue-2229557186

ggwpez commented 5 months ago

I dont get what is going on here. What are your trying to do with PJS and subxt?
This should be a runtime-only change.

ltfschoen commented 5 months ago

I guess I should just start by trying to write a unit test within Polkadot SDK that replicates the RPC call error Call: failed decoding identity.setIdentity due to taking the input of an email address that's too long to setIdentity(info) where the email Data::Raw value is say abcdefghijklmnopqrstuvwxyz@me.com, and then try to change the Polkadot SDK identity runtime implementation until that test passes when running cargo test -p pallet-identity.

I was effectively trying to write the equivalent of an e2e test instead with an external tool like PJS or subxt to create a JSON-RPC call to a locally running Polkadot SDK Substrate node with development environment debugging logs activated so I could debug and know whether my changes to the Polkadot SDK identity runtime implementation actually work and solve the problem.

ggwpez commented 5 months ago

Okay i see. You can first do the changes with a unit test of setIdentity (which should be enough testing in itself), and then if you still want to try it manually once just run with local node and PJS.
We dont have integration testing with PJS, subxt or RPC stuff for normal runtime changes.

ltfschoen commented 4 months ago

Ok thanks, I'm working on it in this PR https://github.com/paritytech/polkadot-sdk/pull/4043/files. I'm not sure where to start with the implementation though