Skip to content
This repository has been archived by the owner on Apr 15, 2020. It is now read-only.

suggest language changes, order of arguments #24

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

nmushegian
Copy link

at dapphub we're taking a look at this, we've wanted something like this forever. This PR is some suggested changes, the most important one is changing the argument order, purely for aesthetic / usability reasons. The concept of registrationIdentifier is replaced with a key, as in simple key-value pairs you can write "about" anyone.

open issues:

  • from/to could have clearer names (but I prefer this graph analogy over issuer and subject)
  • put APL in the source? Good for when contract source file is uploaded to explorers

@nmushegian
Copy link
Author

We could make Set anonymous which would let us index value. This gives you a lot more mileage out of the key space, some apps could make use of just a single key

@oed
Copy link
Contributor

oed commented Apr 2, 2017

Thanks, looks like good improvements. Should be no problems adopting this if it's not to much work to change in our libs etc. I moved the PR to be against develop instead of master.

Could you elaborate on what anonymous does to events? The solidity documentation is not so informative.

@oed oed changed the base branch from master to develop April 2, 2017 09:18
@nmushegian
Copy link
Author

nmushegian commented Apr 2, 2017

Each event actually gets 4 indices, but solidity uses one automatically for the event signature. This is why you can do registry.Set.watch(callback), it's listening to events and filtering by the Set(...) signature just like a function ABI. The anonymous modifier disables this auto-index, freeing you to use all 4 slots. You can see how the Set event has 4 indexed args, and the contract still compiles. There's only one event on this contract, so there's almost no downside, you just watch all events from the contract instead of trying to filter by signature.

@zmitton
Copy link
Contributor

zmitton commented Apr 5, 2017

@nmushegian the naming of issuer subject was chosen because it relates to a lot of other work that has been done in the self-sovereign identity space. There are many people working on verifiable-claims, credentials and attestations. Its best to adopt their very specific language.

Thank you for the tip using anonymous, we can likely add this to the next released version. Can you describe some use cases of filtering by value?

The naming of key vs registrationIdentifier is reasonable. This key will have a few differing purposes which we will publicly outline soon. @oed @christianlundkvist what do you think. I believe we can merge this specific change ( key vs registrationIdentifier), without affecting the bytecode. If so I'm willing to do so. It sounds more approachable and is an very flexible word.

@nmushegian
Copy link
Author

@zmitton Note that changing the argument order definitely will change the bytecode. It's kind of a personal aesthetics thing, but it feels much cleaner to think of both issuer/subject addresses as "higher order" indices than key/value. I think changing registrationIdentifier to key could make sense on its own though.

A basic example for indexing by value is to have reverse lookups. For example if I can mark key: alias, value: 0xYour_Aliasabout you. Then you might want to say "who is everyone who thinks 0xYour_Alias is an alias for me?".

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants