Why can't the value in add_attribute be “”?

[99Kies]

I think the need for event's value to be "" should still be quite high.
https://github.com/CosmWasm/wasmd/blob/main/x/wasm/keeper/events.go#L57-L61

CosmWasm/cosmwasm#1869

#1618

I don't know why it can't be "", I think it's still necessary to remove that part of the code.

If it's a judgment about add_attribute not being able to be "" it shouldn't be in wasmd, you should put it in cosmwasm-std.
https://github.com/CosmWasm/cosmwasm/blob/7b2e753e4752eeb2466a7411ec8e177147581c40/packages/std/src/results/events.rs

[99Kies]

If you want to add judgments, shouldn't they all be added in cosmwasm-std, why here.
https://github.com/CosmWasm/wasmd/blob/main/x/wasm/keeper/events.go#L53-L61

https://github.com/CosmWasm/cosmwasm/blob/7b2e753e4752eeb2466a7411ec8e177147581c40/packages/std/src/results/events.rs#L70-L117

[99Kies]

It's a very strange limitation. This is my current code.

pub fn execute_set_round_info(
    deps: DepsMut,
    _env: Env,
    info: MessageInfo,
    round_info: RoundInfo,
) -> Result<Response, ContractError> {
    if !can_execute(deps.as_ref(), info.sender.as_ref())? {
        Err(ContractError::Unauthorized {})
    } else {
        ROUNDINFO.save(deps.storage, &round_info)?;

        let mut attributes = vec![];
        if round_info.title != "" {
            attributes.push(attr("title", round_info.title))
        }

        if round_info.description != "" {
            attributes.push(attr("description", round_info.description))
        }

        if round_info.link != "" {
            attributes.push(attr("link", round_info.link))
        }

        Ok(Response::new().add_attributes(attributes))
    }
}

[alpe]

Thanks for bringing this up for discussion. Can you elaborate a bit more on your use case?

[99Kies]

@alpe I need to add relevant title, description and link information to my Proposal. Since I have a cosmwasm-indexer synchronization script under the chain, I will synchronize the latest data to the database based on the event that is triggered. This way the front end can directly access the back end api to get the data.

[99Kies]

@alpe In addition to the need that the indexer will depend on the event, the event will also be displayed in the transaction body, and for some set operations, I think the user will want to know the final result. (These results can only be displayed in event, and some of them will definitely be "" as well)

such as: set_description(description: string), set_note(note: string), set_img_url(img_url: string) etc.

[alpe]

I am not sure if I got this correct but in order to export your data from the contract to the DB you store the attributes 1-to-1 with a script. Wouldn't it be simpler to just do a check in the script if the attribute is set and then store it? Or use a defined null value to workaround the issue?
In the code snippet above you were using raw attributes for the data but no event identifier. Do you know that you can use custom event types? This may enable a better mapping for the attributes to the DB store. Empty attributes would still not be allowed.
Supporting empty attribute values is a breaking change. I would like to understand better why they are required for your use case.

[lshoo]

i think the event is a key/value pair, if the value is null, don't emit the event

[webmaster128]

I agree with @99Kies here. It is irritating that an attribute value cannot be empty. You can easily have applications where an attribute value may be empty and then it is nice to just emit it instead of having to add extra logic to omit it.

This problem is not easily visible during the contract development process and only pops up late when the contract is on chain. Even worse, it can remain undetected for a long time because usually a field is set and then cause transaction failures when the value is empty.

I think it would be good to just verify that empty attribute values are not causing an issue in the SDK and then remove the check in the next consensus-breaking release.

i think the event is a key/value pair, if the value is null, don't emit the event

An event contains a list of attributes (i.e. a list of key/value pairs). Not emitting the event has a different meaning than not adding an attribute to the list of an event.

[alpe]

ok, let's accept empty values. I don't have the client in mind but the contract devs that may not check the value before passing it to the event. I saw this recently and thought about this again.

[alpe]

There are also whitespaces allowed in the PR #1618. I assume it is the same argument to support them.

[webmaster128]

Whitespace is trimmed away in all cases. Then I think it's consistent to do that in whitespace-only cases as well. This diff in the PR looks good to me.