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

Reposts with deletions/edits V2 #3697

Closed
wants to merge 13 commits into from
Closed

Conversation

moodyjon
Copy link
Contributor

@moodyjon moodyjon commented Oct 24, 2022

Context: lbryio/types#49

This based on a different claim.proto schema that limits modifications to only reposts of streams and only within a specific set of "Stream.Extension" fields. This is enforced at the level of the protobuf.

The Stream.Extension message contains a protobuf.Any. Right now I check the type name to make sure it's a type that 1) is known when the SDK is built, and 2) comes from a limited list of messages that are intended to be used as extensions. But I think it would be possible to put a claim ID as the "type_url_prefix" of protobuf.Any and download the "descriptor" needed to understand the type. For example, if the "@type" field of the Any message contains "<claim_id>/pb.Cad", the SDK can try to download the contents pointed to by <claim_id>.

To see the claim.proto changes go here:
https://github.com/moodyjon/types/commits/repost_modify

Hub changes that would index the extension fields are TODO.

@lbry-bot lbry-bot assigned moodyjon and lyoshenka and unassigned moodyjon Oct 24, 2022
@moodyjon moodyjon marked this pull request as draft October 25, 2022 02:07
detect whether a repost actually contains modifications.
@lyoshenka
Copy link
Member

leaving a note since im assigned as reviewer: we decided to go with a simple string->[]string mapping instead of protobuf.Any

@lyoshenka lyoshenka removed their request for review November 3, 2022 20:38
lbry/extras/daemon/daemon.py Outdated Show resolved Hide resolved
lbry/extras/daemon/daemon.py Outdated Show resolved Hide resolved
lbry/schema/attrs.py Outdated Show resolved Hide resolved
@moodyjon
Copy link
Contributor Author

moodyjon commented Nov 3, 2022

State of this is I need to remove the dynamic extension type stuff, and add some code to catch StreamExtensionTypeUnresolved when an unknown extension is encountered. For example, a stream claim has some extensions only some of which are of known type. We would want to ignore the unknown ones.

@moodyjon
Copy link
Contributor Author

moodyjon commented Nov 3, 2022

To see the claim.proto changes go here:
https://github.com/moodyjon/types/commits/repost_modify

The latest is I am storing extensions in a map instead of a repeated field. (Got that idea from: #2983)

    // Value should contain one of the extension types (e.g. StringMap).
    // Key provides a hint indicating what keys we should expect in the map.
    map<string, google.protobuf.Any> extensions = 14;

For these StringMap extensions, the key of the map entry is what I was previously placing in StringMap.schema.
(For resource-like things, the key would be the resource name).

Also, the values in StringMap are lists of strings. (Got that idea from: https://github.com/bb-msf-dd/types/pull/1/files#diff-4d1a25cbe9fc3b724c4dd38caf452672c542838cdc0cd567dc12460804534fc5R97)

// StringMap is a generic kind of Stream.Extension (see claim.proto) which
// is useful for testing, but is not space-efficient and does not enforce a
// limited set of fields.
message StringMap {
    map<string, StringList> s = 2;
}

message StringList {
    repeated string vs = 1;
}

@moodyjon
Copy link
Contributor Author

moodyjon commented Nov 8, 2022

Latest .proto changes look like:

message Stream {
    Source source = 1;
    string author = 2;
    string license = 3;
    string license_url = 4;
    int64 release_time = 5; // seconds since UNIX epoch
    Fee fee = 6;
    oneof type {
        Image image = 10;
        Video video = 11;
        Audio audio = 12;
        Software software = 13;
    }

    // Map key is a label for the extension hinting what info we should expect.
    // For example, one could have 2 extensions, "ebook" and "literature", which
    // are both StringMaps, but contain different sets of keys.
    // Map value should contain one of the extension types (e.g. StringMap)
    // encoded as a protobuf.Any.
    map<string, google.protobuf.Any> extensions = 14;

    message Modifiable {
        // Limit modifications to extensions for now.
        map<string, google.protobuf.Any> extensions = 1;
    }
}

message ModifyStream {
    Stream.Modifiable deletions = 1;
    Stream.Modifiable edits = 2;
}

message ModifyingClaimReference {
    bytes claim_hash = 1;
    oneof type {
        ModifyStream stream = 2;
        // Limit modifications to streams for now.
    }
}
// StringMap is a generic kind of Stream.Extension (see claim.proto) which
// is useful for testing & prototyping, but is not space-efficient and does
// not enforce a fixed set of fields.
message StringMap {
    message Value {
        repeated string vs = 1;
    }
    map<string, Value> s = 1;
}

@moodyjon moodyjon marked this pull request as ready for review November 8, 2022 18:53
@moodyjon moodyjon marked this pull request as draft November 9, 2022 16:06
@moodyjon
Copy link
Contributor Author

moodyjon commented Nov 9, 2022

Planning some further simplification after talking to @lyoshenka.

@moodyjon
Copy link
Contributor Author

Closed in favor of #3706

@moodyjon moodyjon closed this Nov 10, 2022
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.

3 participants