bmresearch / Solnet

Solana's .NET SDK and integration library.
https://blockmountain.io/Solnet
MIT License
324 stars 130 forks source link

[Bug] AccountMeta does not match what it should be #152

Closed Arrowana closed 3 years ago

Arrowana commented 3 years ago

Describe the bug The code interface is awkward or incorrect, when an account need to be a signer, Account is provided https://github.com/bmresearch/Solnet/blob/93b568458abe5f866d0e030ac0fe1369f32e2d03/src/Solnet.Rpc/Models/AccountMeta.cs#L41-L48

I think that is quite a big change that makes it misaligned to how the rust solana_sdk or web3.js works. This also destroys the opportunity to build the transaction with C# then provide it to another component to be signed, as any signer need to be provided at instruction construction. So impossible to achieve multisig from different physical places, signing by hardware wallet...

It is quite important to respect the same interface between abstractions to also avoid a steeper learning curve between each abstraction.

I propose to just do it like web3.js https://solana-labs.github.io/solana-web3.js/modules.html#AccountMeta Another option is to follow the rust way, with a static method that sets one of the properties AccountMeta.readOnly(publicKey, isSigner)

Then providing Signer[] is done at the transaction level

hoakbuilds commented 3 years ago

It has been a while since we refactored programs to be like that and the goal of it was for the abstraction to have a lower learning curve, not a steeper one. As this allowed us to build transactions and not even to worry about the order of the signers when passing them into the TransactionBuilder.Build().

OTOH most of our program implementations so far have been from looking at their rust implementation so, from my point of view it would make it easier and overall make more sense to have our implementation be similar to the ::new_readonly() it uses.

I'm gonna give this some more thought and see if there's any way to keep the TransactionBuilder as is, but it looks like this is a major breaking change at various levels.