technicallyty
    • Create new note
    • Create a note from template
      • Sharing URL Link copied
      • /edit
      • View mode
        • Edit mode
        • View mode
        • Book mode
        • Slide mode
        Edit mode View mode Book mode Slide mode
      • Customize slides
      • Note Permission
      • Read
        • Only me
        • Signed-in users
        • Everyone
        Only me Signed-in users Everyone
      • Write
        • Only me
        • Signed-in users
        • Everyone
        Only me Signed-in users Everyone
      • Engagement control Commenting, Suggest edit, Emoji Reply
    • Invite by email
      Invitee

      This note has no invitees

    • Publish Note

      Share your work with the world Congratulations! 🎉 Your note is out in the world Publish Note

      Your note will be visible on your profile and discoverable by anyone.
      Your note is now live.
      This note is visible on your profile and discoverable online.
      Everyone on the web can find and read all notes of this public team.
      See published notes
      Unpublish note
      Please check the box to agree to the Community Guidelines.
      View profile
    • Commenting
      Permission
      Disabled Forbidden Owners Signed-in users Everyone
    • Enable
    • Permission
      • Forbidden
      • Owners
      • Signed-in users
      • Everyone
    • Suggest edit
      Permission
      Disabled Forbidden Owners Signed-in users Everyone
    • Enable
    • Permission
      • Forbidden
      • Owners
      • Signed-in users
    • Emoji Reply
    • Enable
    • Versions and GitHub Sync
    • Note settings
    • Note Insights New
    • Engagement control
    • Make a copy
    • Transfer ownership
    • Delete this note
    • Save as template
    • Insert from template
    • Import from
      • Dropbox
      • Google Drive
      • Gist
      • Clipboard
    • Export to
      • Dropbox
      • Google Drive
      • Gist
    • Download
      • Markdown
      • HTML
      • Raw HTML
Menu Note settings Note Insights Versions and GitHub Sync Sharing URL Create Help
Create Create new note Create a note from template
Menu
Options
Engagement control Make a copy Transfer ownership Delete this note
Import from
Dropbox Google Drive Gist Clipboard
Export to
Dropbox Google Drive Gist
Download
Markdown HTML Raw HTML
Back
Sharing URL Link copied
/edit
View mode
  • Edit mode
  • View mode
  • Book mode
  • Slide mode
Edit mode View mode Book mode Slide mode
Customize slides
Note Permission
Read
Only me
  • Only me
  • Signed-in users
  • Everyone
Only me Signed-in users Everyone
Write
Only me
  • Only me
  • Signed-in users
  • Everyone
Only me Signed-in users Everyone
Engagement control Commenting, Suggest edit, Emoji Reply
  • Invite by email
    Invitee

    This note has no invitees

  • Publish Note

    Share your work with the world Congratulations! 🎉 Your note is out in the world Publish Note

    Your note will be visible on your profile and discoverable by anyone.
    Your note is now live.
    This note is visible on your profile and discoverable online.
    Everyone on the web can find and read all notes of this public team.
    See published notes
    Unpublish note
    Please check the box to agree to the Community Guidelines.
    View profile
    Engagement control
    Commenting
    Permission
    Disabled Forbidden Owners Signed-in users Everyone
    Enable
    Permission
    • Forbidden
    • Owners
    • Signed-in users
    • Everyone
    Suggest edit
    Permission
    Disabled Forbidden Owners Signed-in users Everyone
    Enable
    Permission
    • Forbidden
    • Owners
    • Signed-in users
    Emoji Reply
    Enable
    Import from Dropbox Google Drive Gist Clipboard
       Owned this note    Owned this note      
    Published Linked with GitHub
    • Any changes
      Be notified of any changes
    • Mention me
      Be notified of mention me
    • Unsubscribe
    # Pulsar WG 10/6/21 - [x] Frojdi's interface service - [ ] Tyler's proto methods - https://github.com/cosmos/cosmos-proto/blob/ty/proto_methods/features/fastreflection/proto_message.go#L284 https://github.com/cosmos/cosmos-proto/blob/ty/proto_methods/testpb/1.pulsar.go#L973 - [ ] Backlog grooming ### Notes having context available would be a good thing in interface methods, sometimes not since we need to have context on what the protobuf any representation is, its better to have a wrappper type with private methods, so people cannot create these interfaces without using some specialiazed constructor and check the interface registery to see if its possible to create this interface type or not. it follows the any convention where you cant create an any without a type resolver message any is gonna implement the interface and end up in the codegen msg will be a concrete type which implements the interface constructing this message will require you to pass in the interface and an interface registry. this impl keeps up in a place where we have to be explicit codegen will take care of protomethods for you we will need an any cache the registry will have the interface implemeneter, which will know of the interface implmeneters maybe a global registry is not enough cause certain chains want to accept some implementations and not others advantages: * you get an any cache * (alternative: getters and setters on struct) * having getters and setters makes things easier than having to modify fields * wrapping AnyMsg makes more sense theres a new constructor that populates a private field tx object stateful with interface registry there? (option) aaron: make the behavior consistent if we say the interface registry is only gonna check on marshal but not protoreflection, as long as we are consistent, its probably not a problem frojdi: agree with that approach, if we say you are gonna work with the concrete type, you'll be safe, but if you work with ProtoReflect, you have to be sure the type you are gonna be setting is an interface implementer but you cannot be sure that the interface registry will accept it or not. thats what we can assure to other devs aaron: if we use adr 33, this interface registry isnt checked. it doesnt apply to inter module communication. frojdi: its only proto reflect set. im not seeing a context in which devs are using protoreflect set, when you are sending messages youre sending concrete types not protoreflect types. for adr 33 its a non-issue. its only going to be an issue for those modules or anything that is working with arbitrary proto types. i feel its a non-issue. aaron: there are some tradeoffs in this approach. it makes sense but also its hard to make everyone happy, sometimes people dont understand why things are done a certain way and frustrated it wasnt done another way. frojdi: it needs to be clear its not possible to get best of both worlds. need some form of tradeoff. either have complete safety, or bad dev ux, choices. we could keep things the way they are right now and it'll be safe right now. but people can still set arbitrary anys in interfaces, and we are gonna detect that only when we unmarshal/marshaling. but here we provide some protection when setting, but we dont have protection against protoreflect. but we are improving things, not making them worse sam: one thing i think could be useful - be clear about who is subject to the bad parts - devs/users/things that use the sdk. who does this affect? frojdi: you are getting unsafety with protoreflect, aaron: its library devs. its up to you to check the interface registry when using the code frojdi: we can put in the codec a check for the interface. we can use protobuf options to determine where the interface is and if its a correct option or not. we can abstract it away. its an advanced use case. only place we'll see this is ORM. even ORM doesnt even need to do this check cause it'll marshal bytes. marshal will check for it. its a problem if you're using protoreflect and you're not gonna marshal after. aaron: another side to consider, using any message wrapper, but not returning an error and not returning a registry. we could not check it until marshal. frojdi: if you do that you'll have adr 33 problems. having that error check is gonna protect you because in this case you have no way of setting this thing without like using a specialized constructor which accepts an interface registry. you have no way of doing this, so you are safe here. you are unsafe only if you do this thing: msg.Protoreflect().Set(...) aaron: say theres a module that creates a vesting account, calls another module to create a vesting account, but that vesting account not supported on chain, once a client tries to unmarshal it, the vesting account wont work. frojdi: you are saying we'll have this problem if you are trying to marshal/unmarshal a vesting account, the concrete type is not gonna give issues aaron: im trying to see what would be the problem if we didnt deal with the interface registry in the any message wrapper. frojdi: you could have this happening - frojdi: you're dealing with things the state machine should not be dealing with. unexpected beahavior. having a constructor which always chcks if the chain supports that type is most likely better. aaron: it makes sense frojdi: its not the best devx but its better than what we have right now aaron: its an imporvement and more correct. correct by default increase devx even if its not obvious aaron: interface in the sdk. if we're moving in the direction of having the protobuf types in the go module, and we're saying this is just the types modules, and its just a version of the protobuf files codegen'd. theres a bit of a challenge with interfaces where the interfaces are implmentation. and we are putting the implmentation in the same package as the generated code. is there a way we can separate the impl of these interfaces from the concrete proto types that are used to represent them? could we have a msg send proto type that implments a msg service interface *outside* of that package. we could have multiple impl. of that service. frojdi: you'd need to separate functions that are impl the interface from the type itself, then teh concern of how you are calling those methods, cause you cannot call directly on the type itself. so idk, it would be counter intuitive from how we deal with interfaces in golang. aaron: if we are using a wrapper and you pass the registry to the wrapper, you could pull the impl into the wrapper type and then you have both conc. type and impl. theres ways you could make it better frojdi: the only thing that would look weird is how you are actually implmenting these interfaces. you are not just registring the type but the functions that execute on the type. if you wanna separate conc. type from interface logic. it could be done, but most likely for devs it would be hard to understand aaron: is there value to the concern that you are tightly tied to the interface impl. - it feels a bit fragile from a semantic versioning persspective. frojdi: i agree with this. even in general/sdk i would not push people to make interface types with anys. i would avoid them as much as possible. in the sdk context where we deal with types as interfaces, we can deal with them a different way. for exmaple crisis. even evidence handlers could be handlers. we are sending a message that is an evidence and a separate handler that is dealing with that message. there are contexts where it might be useful to have interface and have diff impls. but i would try to avoid them as much as possibe. i would avoid interfaces as much as possible for sdk types. i see a use case for crypto keys, accounts, but there are also a lot of use cases where we can deal without interfaces, such as crisis, evidence. they could just be handlers. handlers of messages. rather than messages that implement an inteface which is a handler. aaron: messages in the transaction,they have handlers. but we decided to pull ou these things, one is get signers, validate basic. there was both handler and a something defined on the concrete type. instead of interface its just a handler. frojdi: in favor of that approach, i prefer it over everything else. even if ou take the most obvious example, which is sdk.Msg with validate basic, validate basic in my book is just a handler that accepts that message and validates it. the message should not implmenet the validation handler itself. in that context, you would enable the use case where you might have the same message where you oculd have multiple handlers on the same message, idk how you will pick which one to use. but yeah that would be a better approach aaron: you could have those in the interface registry and they would always get registered frojdi: how are yo ugonna call it from a message. are you using some sort of client? aaron: current validate basic is fragile. get signers is one of the reasons we dont have a good client ux. we cant have some of the good things we want like semantic versioning. get signers requires a global. the whole message interfaces, theres reasons it is this way. lets just fix those problems and design it the right way frojdi: couldnt you simply mark which fields need to be signed with protobuf options? the only problem you can not do and/or/if logic. we should limit use of interfaces use interface with things that actually need them

    Import from clipboard

    Paste your markdown or webpage here...

    Advanced permission required

    Your current role can only read. Ask the system administrator to acquire write and comment permission.

    This team is disabled

    Sorry, this team is disabled. You can't edit this note.

    This note is locked

    Sorry, only owner can edit this note.

    Reach the limit

    Sorry, you've reached the max length this note can be.
    Please reduce the content or divide it to more notes, thank you!

    Import from Gist

    Import from Snippet

    or

    Export to Snippet

    Are you sure?

    Do you really want to delete this note?
    All users will lose their connection.

    Create a note from template

    Create a note from template

    Oops...
    This template has been removed or transferred.
    Upgrade
    All
    • All
    • Team
    No template.

    Create a template

    Upgrade

    Delete template

    Do you really want to delete this template?
    Turn this template into a regular note and keep its content, versions, and comments.

    This page need refresh

    You have an incompatible client version.
    Refresh to update.
    New version available!
    See releases notes here
    Refresh to enjoy new features.
    Your user state has changed.
    Refresh to load new user state.

    Sign in

    Forgot password

    or

    By clicking below, you agree to our terms of service.

    Sign in via Facebook Sign in via Twitter Sign in via GitHub Sign in via Dropbox Sign in with Wallet
    Wallet ( )
    Connect another wallet

    New to HackMD? Sign up

    Help

    • English
    • 中文
    • Français
    • Deutsch
    • 日本語
    • Español
    • Català
    • Ελληνικά
    • Português
    • italiano
    • Türkçe
    • Русский
    • Nederlands
    • hrvatski jezik
    • język polski
    • Українська
    • हिन्दी
    • svenska
    • Esperanto
    • dansk

    Documents

    Help & Tutorial

    How to use Book mode

    Slide Example

    API Docs

    Edit in VSCode

    Install browser extension

    Contacts

    Feedback

    Discord

    Send us email

    Resources

    Releases

    Pricing

    Blog

    Policy

    Terms

    Privacy

    Cheatsheet

    Syntax Example Reference
    # Header Header 基本排版
    - Unordered List
    • Unordered List
    1. Ordered List
    1. Ordered List
    - [ ] Todo List
    • Todo List
    > Blockquote
    Blockquote
    **Bold font** Bold font
    *Italics font* Italics font
    ~~Strikethrough~~ Strikethrough
    19^th^ 19th
    H~2~O H2O
    ++Inserted text++ Inserted text
    ==Marked text== Marked text
    [link text](https:// "title") Link
    ![image alt](https:// "title") Image
    `Code` Code 在筆記中貼入程式碼
    ```javascript
    var i = 0;
    ```
    var i = 0;
    :smile: :smile: Emoji list
    {%youtube youtube_id %} Externals
    $L^aT_eX$ LaTeX
    :::info
    This is a alert area.
    :::

    This is a alert area.

    Versions and GitHub Sync
    Get Full History Access

    • Edit version name
    • Delete

    revision author avatar     named on  

    More Less

    Note content is identical to the latest version.
    Compare
      Choose a version
      No search result
      Version not found
    Sign in to link this note to GitHub
    Learn more
    This note is not linked with GitHub
     

    Feedback

    Submission failed, please try again

    Thanks for your support.

    On a scale of 0-10, how likely is it that you would recommend HackMD to your friends, family or business associates?

    Please give us some advice and help us improve HackMD.

     

    Thanks for your feedback

    Remove version name

    Do you want to remove this version name and description?

    Transfer ownership

    Transfer to
      Warning: is a public team. If you transfer note to this team, everyone on the web can find and read this note.

        Link with GitHub

        Please authorize HackMD on GitHub
        • Please sign in to GitHub and install the HackMD app on your GitHub repo.
        • HackMD links with GitHub through a GitHub App. You can choose which repo to install our App.
        Learn more  Sign in to GitHub

        Push the note to GitHub Push to GitHub Pull a file from GitHub

          Authorize again
         

        Choose which file to push to

        Select repo
        Refresh Authorize more repos
        Select branch
        Select file
        Select branch
        Choose version(s) to push
        • Save a new version and push
        • Choose from existing versions
        Include title and tags
        Available push count

        Pull from GitHub

         
        File from GitHub
        File from HackMD

        GitHub Link Settings

        File linked

        Linked by
        File path
        Last synced branch
        Available push count

        Danger Zone

        Unlink
        You will no longer receive notification when GitHub file changes after unlink.

        Syncing

        Push failed

        Push successfully