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

feat(schema)!: prepend module to declarations in BorshSchema #300

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dj8yfo
Copy link
Collaborator

@dj8yfo dj8yfo commented Jun 5, 2024

sketch of possible solution to #92, both the original issue and the widened perspective

this results in

    #[derive(borsh::BorshSchema)]
    enum A {
        Bacon(Vec<i64>),
        Eggs,
    }

=>

{
    "Vec<i64>": Sequence {
        length_width: 4,
        length_range: 0..=4294967295,
        elements: "i64",
    },
    "i64": Primitive(
        8,
    ),
    "tests::schema::test_simple_enums::A": Enum {
        tag_width: 1,
        variants: [
            (
                0,
                "Bacon",
                "tests::schema::test_simple_enums::___ABacon",
            ),
            (
                1,
                "Eggs",
                "tests::schema::test_simple_enums::___AEggs",
            ),
        ],
    },
    "tests::schema::test_simple_enums::___ABacon": Struct {
        fields: UnnamedFields(
            [
                "Vec<i64>",
            ],
        ),
    },
    "tests::schema::test_simple_enums::___AEggs": Struct {
        fields: Empty,
    },
}

this may have very weak interoperability with borsh-js due to being specific to rust

@dj8yfo dj8yfo changed the title feat!: prepend module to declarations in BorshSchema feat!(schema): prepend module to declarations in BorshSchema Jun 5, 2024
@dj8yfo dj8yfo changed the title feat!(schema): prepend module to declarations in BorshSchema feat(schema)!: prepend module to declarations in BorshSchema Jun 5, 2024
@dj8yfo
Copy link
Collaborator Author

dj8yfo commented Jun 5, 2024

similarly

#[derive(BorshSchema)]
enum A {
    Bacon(Vec<i64>),
    Eggs,
}

from schema_full_path_crate
results in

{
    "Vec<i64>": Sequence {
        length_width: 4,
        length_range: 0..=4294967295,
        elements: "i64",
    },
    "i64": Primitive(
        8,
    ),
    "schema_full_path_crate::A": Enum {
        tag_width: 1,
        variants: [
            (
                0,
                "Bacon",
                "schema_full_path_crate::___ABacon",
            ),
            (
                1,
                "Eggs",
                "schema_full_path_crate::___AEggs",
            ),
        ],
    },
    "schema_full_path_crate::___ABacon": Struct {
        fields: UnnamedFields(
            [
                "Vec<i64>",
            ],
        ),
    },
    "schema_full_path_crate::___AEggs": Struct {
        fields: Empty,
    },
}

@dj8yfo
Copy link
Collaborator Author

dj8yfo commented Jun 5, 2024

and then,

use schema_full_path_crate::A as AForeign;
#[derive(BorshSchema)]
struct B {
    x: A,
    y: AForeign,
}

from sample_schema_two_crates
results in

BorshSchemaContainer {
    declaration: "sample_schema_two_crates::B",
    definitions: {
        "Vec<i64>": Sequence {
            length_width: 4,
            length_range: 0..=4294967295,
            elements: "i64",
        },
        "i64": Primitive(
            8,
        ),
        "sample_schema_two_crates::A": Enum {
            tag_width: 1,
            variants: [
                (
                    0,
                    "UnitVariant",
                    "sample_schema_two_crates::___AUnitVariant",
                ),
            ],
        },
        "sample_schema_two_crates::B": Struct {
            fields: NamedFields(
                [
                    (
                        "x",
                        "sample_schema_two_crates::A",
                    ),
                    (
                        "y",
                        "schema_full_path_crate::A",
                    ),
                ],
            ),
        },
        "sample_schema_two_crates::___AUnitVariant": Struct {
            fields: Empty,
        },
        "schema_full_path_crate::A": Enum {
            tag_width: 1,
            variants: [
                (
                    0,
                    "Bacon",
                    "schema_full_path_crate::___ABacon",
                ),
                (
                    1,
                    "Eggs",
                    "schema_full_path_crate::___AEggs",
                ),
            ],
        },
        "schema_full_path_crate::___ABacon": Struct {
            fields: UnnamedFields(
                [
                    "Vec<i64>",
                ],
            ),
        },
        "schema_full_path_crate::___AEggs": Struct {
            fields: Empty,
        },
    },
}

@dzmitry-lahoda
Copy link
Contributor

dzmitry-lahoda commented Jun 5, 2024

Seems only this solution will work with 3rd party crates. In my crate I have solutions to rename or use type alias, so not a problem.

Question,

  1. what would be in case of reexport of exactly same data structure by two crates? Two separate declaration with same definitions I assume? What would be with when I use std::Vec and alloc::Vec for example?

  2. Should be schema Rust based expression(syn parseable alike?) or readable by more people(TS?) ? Proposed naming for variants is schema_full_path_crate::___ABacon, but should it be just schema_full_path_crate::A::Bacon? Assuming mod name is not allowed to conflict to variant name in Rust.

@dj8yfo
Copy link
Collaborator Author

dj8yfo commented Jun 6, 2024

@dzmitry-lahoda

Two separate declaration with same definitions I assume?

Yes. But if the types use semantically identical types from their respective crates as their fields, then they will technically
have different definitions as well, because they will be comprised of similar types but with different Declarations, due to crate+mod prefix.

Should be schema Rust based expression(syn parseable alike?) or readable by more people(TS?) ?

The triple underscore prefix (___) for enum helper structs can be removed to improve readability, as 1. it leaks implementation details outside, 2. it's not that big of a problem to arrange to avoid conflicts in a single module.

But then, crate+mod prefix leaks implementation detail outside as well, so this pr most likely won't be merged.

when I use std::vec::Vec and alloc::vec::Vec for example?

It's the same type

@dj8yfo
Copy link
Collaborator Author

dj8yfo commented Jun 6, 2024

@dzmitry-lahoda , also, conflict detection of names as of now cannot be made perfect, at least because recursive types were allowed, so someone can accidentally or in an intentfull way replace or "shadow" someone else's declaration, if many crates are involved and very complex schemas are constructed.
Automatically adding crate+mod prefix can help with accidental cases, but can do little in cases if it's done purposefully.
It should be the same for identical source code versions, so at least that's a great relief.

@dj8yfo dj8yfo marked this pull request as ready for review July 5, 2024 21:12
@dj8yfo dj8yfo requested a review from frol as a code owner July 5, 2024 21:12
@dj8yfo dj8yfo marked this pull request as draft July 5, 2024 21:13
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