Skip to content
This repository has been archived by the owner on Jan 24, 2022. It is now read-only.

Consider indexing name argument in all Vouching events #132

Closed
tinchoabbate opened this issue Sep 25, 2018 · 2 comments
Closed

Consider indexing name argument in all Vouching events #132

tinchoabbate opened this issue Sep 25, 2018 · 2 comments
Assignees
Labels
kind:enhancement Enhancement to an existing feature
Milestone

Comments

@tinchoabbate
Copy link
Contributor

@spalladino suggested in PR #115 indexing name arguments for all events fired in the Vouching contract.

I'm moving the discussion outside the PR so it can be better documented.

Related issues were the problem of indexing string arguments has been discussed:

@tinchoabbate tinchoabbate self-assigned this Sep 25, 2018
@tinchoabbate tinchoabbate added kind:discussion kind:enhancement Enhancement to an existing feature labels Sep 25, 2018
@spalladino
Copy link
Contributor

Following the discussion in OpenZeppelin/openzeppelin-contracts#992, it'd seem like the best course of action would be to make name indexed in all events, and add a fullname unindexed argument in DependencyCreated. WDYT?

@facuspagnuolo facuspagnuolo added this to the v2.0.0 milestone Sep 26, 2018
@tinchoabbate
Copy link
Contributor Author

Reviewed the comment you left in the PR, and I've been working locally on a new implementation that addresses them in some ways.

Quick question, do we need the name to be a string? Could it instead be a bytes32? In my tests I'm seeing that (as you mentioned in the PR), bytes32 indexed event arguments are not being hashed. Furthermore, it seems that only fixed-size types can be indexed (see trufflesuite/truffle#405 (comment)).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind:enhancement Enhancement to an existing feature
Projects
None yet
Development

No branches or pull requests

3 participants