search

vulcanize / go-codec-dagcosmos

A Go implementation of the DAG interface for Tendermint and Cosmos types
Other
8 stars 2 forks source link

IPLD Bind Node #44

Open Wondertan opened 2 years ago

Wondertan commented 2 years ago

There is a relatively new development in IPLD called bindnode which allows binding the existing Golang structs to IPLD schema via reflection and without code generation. It is also a preferred way right now as per IPLD team. Any plans on migrating to bindnode and are there any known obstacles to doing so with Tendermint types?

i-norden commented 2 years ago

Hey thanks for bringing this up. I've been thinking about bindnode a lot lately, as I think we will want (if not need) to use it (or some similar reflection process) to provide support for arbitrary protobuf types. Now that the API is "stable" we should consider porting everything other, and if we end up needing bindnode for SMT-protobuf support it would be nice to have everything conform. While I am finishing up support for those remaining types I will get a better understanding/appreciation for bindnode and whether it makes sense to migrate everything.

For the types that already have custom implementations, is there a specific advantage/reason you had in mind for migrating to the bindnode approach? Is bindnode the preferred approach because it is the quickest way to implement codecs or also because it is generally better for other reasons? The clear advantage I see, other than simplifying implementation, is that Unwrap gives you a direct method to recover the Go type from the IPLD node whereas currently you need to encode the node and then unmarshal the binary back into the Go type.

Some things that need to be considered before attempting to migrate:

  1. My intuition is that the reflection based approach of bindnode is going to be less performant.
  2. Many of our IPLD schema types do not map directly to an existing Go type, e.g. when the Go type includes fields that do not contribute to the consensus encoding. The implications of this are something I need to look into more, but worth noting.
  3. bindnode doesn't support Unions or Enums, although there are ways of eliminating the use of these which we may already want to consider for other reasons...