GeniusVentures / SuperGenius

MIT License
4 stars 3 forks source link

GeniusWallet integration #58

Closed EduMenges closed 1 month ago

EduMenges commented 2 months ago

To be merged with https://github.com/GeniusVentures/GeniusWallet/pull/77 and https://github.com/GeniusVentures/GeniusSDK/pull/9

itsafuu commented 2 weeks ago

I believe it would be better to return transactions from GeniusSDKGetTransactions in a different way. Propose: First, de-serialize the transaction data with protobuf before returning it. Currently the dashboard/wallet need protobuf to handle reading the data that comes back. This can be done with SGTransactions.pb.h

Second, include the key for each transaction for more easily detecting dupes when refreshing transactions.

Additionally, another method in here for getting blockchain header data/blockchain payload data. Again, de-serialize with SGBlocks.pb.h.

EduMenges commented 2 weeks ago

First, de-serialize the transaction data with protobuf before returning it. Currently the dashboard/wallet need protobuf to handle reading the data that comes back. This can be done with SGTransactions.pb.h

But then how would the wallet read the data? We initially passed the raw protobuf bytes to it because the transactions are a tagged union and the whole desserialization thing is better left up to protobuf. I could create a custom C struct and use that, but I think we would just be re-implementing the protobuf part.

itsafuu commented 2 weeks ago

First, de-serialize the transaction data with protobuf before returning it. Currently the dashboard/wallet need protobuf to handle reading the data that comes back. This can be done with SGTransactions.pb.h

But then how would the wallet read the data? We initially passed the raw protobuf bytes to it because the transactions are a tagged union and the whole desserialization thing is better left up to protobuf. I could create a custom C struct and use that, but I think we would just be re-implementing the protobuf part.

Yes, a custom C struct is pretty much in line with what I was thinking. We're currently in a situation where any app linking to the SDK that wants to read transactions must also have protobuf files to decode the information. Not to mention because the transaction could be a mint, transfer, escrow, we must do a bunch of try catch to check which type of de-serialization we need. Though providing the "key" would allow us to check that before we try to de-serialize.

Either way, I don't know how many use cases of reading transaction data apps linking to the SDK we might have, but expecting anyone who wants to do that to grab .proto files and convert them into an appropriate protobuf reader for the language of their app seems to include a lot of friction.

We may wait to address that later though.

The dashboard still needs those block headers/tx from the SDK for display. So if we're not going to de-serialize right now, you could return those anyway with a function to get blocks, I already have protobuf files in dashboard.

henriqueaklein commented 2 weeks ago

I think as far as friction goes, either the person has to know how to walk around a custom C struct or deserialize proto in their own chosen language. Anyway, the access to transactions and stuff I think will be mostly by our stuff. A game dev will worry only about calling Init, so this change would be mostly for us.

I think we could have the custom C struct in GeniusSDK if it makes it any easier on dart. If we want to provide these APIs in a friction-less manner in other languages we could do what TrustWallet does for Java/Swift, that is to implement higher level APIs that does the desserialization for them.

EduMenges commented 2 weeks ago

We're currently in a situation where any app linking to the SDK that wants to read transactions must also have protobuf files to decode the information.

Yeah, that is a good point, but we also have to consider that not every language has a good C interop (like JavaScript), so maybe working with the C struct would make it harder for these cases. JS is dumb tho, probably the users are going to use SGNS with C#, C++ and other game engine friendly languages.

Another concern is dynamic collections: if we need to use vectors and strings, protobuf handles that part, we just need to pass the raw bytes and de-alloc them later. But if we are going to use a C struct, we would need to either make these collections have a static size or deal with allocation and size definition, which would increase complexity. The user would have to replicate this in their own language so that calling our function actually works.

itsafuu commented 2 weeks ago

In the interest of getting things done quickly, for now I would like to see a method to get the blocks, and we can keep returning protobuf. It would also still be nice to also get the key value for the crdt entry along with these so I can avoid a bunch of try catch blocks for decoding with protobuf.

EduMenges commented 2 weeks ago

for now I would like to see a method to get the blocks

Do you mean to get a whole block from the blockchain?

itsafuu commented 2 weeks ago

for now I would like to see a method to get the blocks

Do you mean to get a whole block from the blockchain?

Yeah, see screenshot I posted a couple days ago to general of the Dashboard, it has "Recent Blocks" and "Recent Tranasactions".

EduMenges commented 2 weeks ago

Yeah, see screenshot I posted a couple days ago to general of the Dashboard, it has "Recent Blocks" and "Recent Tranasactions".

Ok, should we get the block and the header?

itsafuu commented 2 weeks ago

Yeah, see screenshot I posted a couple days ago to general of the Dashboard, it has "Recent Blocks" and "Recent Tranasactions".

Ok, should we get the block and the header?

Yeah, I think in theory it displays the header and you could click on it to get the block info itself. Probably best to have both.