CosmWasm / wasmd

Basic cosmos-sdk app with web assembly smart contracts
Other
367 stars 400 forks source link

Event wasm token id is incorrect when token id has space at first or last position #1206

Closed fibonacci998 closed 1 year ago

fibonacci998 commented 1 year ago

When mint a CW721 token, if I mint a token id abc (has space at first position) or abc(has space at last position), wasm token id event returned is abc. It may be issue because the token ID is not the same as input

fibonacci998 commented 1 year ago

Found in wasm docs: EVENTS.md Why do you need trim space all value? If values is not the same as input, we cannot find this tx when search by wasm.token_id=' abc'

alpe commented 1 year ago

We do not want leading/ trailing whitespaces to provide a better user experience. Events are not purely technical for machine to machine communication but can be read by humans, too. Therefore whitespaces where a big source of confusion in these systems. An alternative solution, would be rejecting them and failing the TX for consistency. Not sure if this better. Maybe you can elaborate more on your business use case to help me understand the problem from your side.

With my current understanding, I see the issue in the cw721 contract which should reject tokens with leading/trailing whitespace instead. There are scam scenarios to prevent.

fibonacci998 commented 1 year ago

With my case, when I use LCD endpoint /cosmos/tx/v1beta1/txs?events= and pass argument wasm.token_id=' abc ', LCD return nothing because currently, wasmd saved event is wasm.token_id='abc'. There is no way to find txs which are interacting with token_id abc

alpe commented 1 year ago

@fibonacci998 Sorry, my question was: why does ' abc ' make sense for you/ your project?

fibonacci998 commented 1 year ago

Example, I'm developing a wallet which allow shows address's NFT. In my database, address A has token_id is ' abc ', address B has token_id is 'abc'. When I access token NFT history tx from address A, I cannot see correct tx by access LCD endpoint /cosmos/tx/v1beta1/txs?events= (because it displays all tx from address A and address B)

alpe commented 1 year ago

@fibonacci998 don't get me wrong. I do understand the query part. Just not the motivation to create a abc token_id in the first place. This is not wasmd related but how you are using the nft contract

fibonacci998 commented 1 year ago

I just don't understand why should you trim the value in event?

alpe commented 1 year ago

@fibonacci998 I have explained our motivation above already. I do understand that this is not the expected behaviour for you. Let's agree to disagree on the solution, unless you can provide more context on your use case and why whitespaces make sense in the token_id.

fibonacci998 commented 1 year ago

Agreed with your solution, not only event is a technical for human read, but also event is a way to search tx in Cosmos SDK. I was stuck about that how to search that tx onchain. I can search tx offchain by adding more column in my database, but it is complex because of database design and my business.

alpe commented 1 year ago

I hope the contract get's fixed soon. This does not help with existing installations but makes things easier in the future for everybody. For your data analysis business, it may also be an option to run a custom node with a modified wasmd version. Events are not part of consensus.