Add ability to setFeePayer, add toString methods #83
Conversation
| val feePayerMeta = keysList[feePayerIndex] | ||
| newList.add(AccountMeta(feePayerMeta.publicKey, true, true)) | ||
| keysList.removeAt(feePayerIndex) | ||
| } catch(e: RuntimeException) { // Fee payer not yet in list |
There was a problem hiding this comment.
I think it's better if findAccountIndex returns -1 when it doesn't find the account instead of throwing. Then this would be an if else and not a try catch. Curious what maintainers think about that change.
There was a problem hiding this comment.
Another option is to implement a custom setter for FeePayer that adds the account to the array when it it set. My concern with that is it wouldn't be trivial to handle a situation like:
tx.setFeePayer(x)
tx.setFeePayer(y)
which would result in an extra account in the array unless some complex logic is implemented to decide if x can be safely removed in the setter.
| @@ -39,7 +43,8 @@ class Message { | |||
| private var recentBlockhash: String? = null | |||
| private val accountKeys: AccountKeysList = AccountKeysList() | |||
| private val instructions: MutableList<TransactionInstruction> | |||
| private var feePayer: Account? = null | |||
| var feePayer: PublicKey? = null | |||
There was a problem hiding this comment.
We need both getter and setter in order to check if we've already set a feePayer before applying the default.
|
I did this for iOS and then remove it. In my opinion the Fee payer is always the first account (I may be incorrect, but this is my understanding about Solana). I am with you in opening the Payer but we might need to manage the case when the payer is not the first. Account. I might be wrong about this The other is that the set Payer is currently an Account not a PublicKey. |
You're correct, and that's already been handled. If you look at my changes to Message.kt, you'll see I'm putting the feePayer account first in the event that it wasn't in the list of accounts. But prior to my change, you already had in that same place
Looks like I missed changing the Account to PublicKey somewhere. I'll take a look at that |
|
I think it can go in. |
| @@ -135,6 +140,15 @@ class Message { | |||
| throw RuntimeException("unable to find account index") | |||
| } | |||
|
|
|||
| override fun toString(): String { | |||
Currently Message.setFeePayer is not exposed to the Transaction. It's only used to automatically set the first signature in the list to FeePayer. Acording to the Solana spec, this is a good default but it should be configurable. This PR adds the ability to specify a fee payer. See below in-line github comments for places it may need to be written differently.
I also added toString methods to all the Transaction components because they made my debugging easier.