Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DSNP Announce Spec #8

Closed
wants to merge 3 commits into from
Closed

Conversation

wilwade
Copy link
Member

@wilwade wilwade commented Feb 22, 2021

Problem

Users of DSNP need a way to publish messages in a standard way
link to Pivotal Tracker #23177049245

Solution

A separate interface for creating an DSNPMessage event that announces a new message.

Change summary:

  • Started a new spec section DSNP Announce
  • Added to the sidebar
  • Referenced DSNPMessage


| field | description | type |
|-------|-------------|------|
| from | Social Identity From Address | address / uint160 / bytes20 |
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Should we have this here?
  2. Should we name it something different?
  3. Should it use the Ethereum address type or do we want to make it generic bytes?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I happen to like the convention either of from or fromAddr as a name.

If it's generic bytes, then how does it get mapped to a social identity contract?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried out the name dsnpFrom and standardized the prefixes. Thoughts?

In Solidity address, uint160, bytes20 are all synonyms. The connection would likely come as part 2. The identity interface likely will be required to implement https://eips.ethereum.org/EIPS/eip-1271 that would allow for verification at the level of the announcing contract and the end user.

Likely that will also require that the signature (bytes32) be included as a core log level data point.

pages/DSNP/DSNP-Announce.md Outdated Show resolved Hide resolved
@wilwade wilwade force-pushed the features/dsnp-annoucing-#177049245 branch from 34b8c2d to 48a6ed3 Compare February 22, 2021 22:07
Copy link
Contributor

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

YUSS

pages/DSNP/DSNP-Announce.md Outdated Show resolved Hide resolved
Copy link
Contributor

@shannonwells shannonwells left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With respect to labeling address types, let's just say "address" in the type and make a note at the top about address/uint160/bytes20

Also I think it would be good to have a short description of what the Announce contract is meant for.

I think I still prefer the "route" idea but there are good non-conflicting things in here that would go in #11 .

pages/DSNP/DSNP-Announce.md Outdated Show resolved Hide resolved

### Announce Requirements

| Interface | Required |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also I like this section. Maybe we should have in the spec template too.

@wilwade wilwade mentioned this pull request Feb 26, 2021
@wilwade wilwade closed this Mar 1, 2021
@wilwade wilwade deleted the features/dsnp-annoucing-#177049245 branch March 2, 2021 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants