Add TransactionBuilderOptions to set default settings & deprecate TransactionProcessor usage #39
Conversation
| this.instructions = []; | ||
| this.signers = []; | ||
| this.opts = defaultOpts ?? defaultTransactionBuilderOptions; |
There was a problem hiding this comment.
Would it be nicer for users of this class to only be able to pass in partial default options? For example, I don't think the following is possible:
const builder = new TransactionBuilder(connection, wallet, {
defaultBuildOption: {
blockhashCommitment: "confirmed"
}
})
There was a problem hiding this comment.
yeah i've been attempting to do that but it's quite difficult to wrestle typescript to have nested partials while having intersection types like we have for BuildOptions.
For now, users would have to build their own by spreading the nested properties in their custom options.
| ...defaultBuildOptions, | ||
| ...options, | ||
| }; | ||
| async txnSize(userOptions?: Partial<BuildOptions>): Promise<number> { |
There was a problem hiding this comment.
Also - unrelated to this PR, but I was thinking it would be nice to make txnSize a synchronous function. It looks like the only async invocation is for getting the latest blockhash when calling build() to create the transaction.
Since the size of the blockhash never changes, what do you think about using a hardcoded blockhash value for transaction size calculations? We can make sure to get the latest blockhash when actually preparing the transaction send.
This is especially nice for a MultiTransactionBuilder I'm working on where it uses txnSize to determine if the current transaction is full, but the async messes up the fluent interface for working w/ the builder.
There was a problem hiding this comment.
agreed that would be nice. can we add it as part of the multi-tx initiative though? the effort would be to separate out the blockhash code from build and create a second "buildSync" method or something.
There was a problem hiding this comment.
Yup, sounds good! Working on it right now actually - I'll open a separate PR
| const msg = txnMsg.compileToV0Message(lookupTableAccounts); | ||
| txnMsg.compileToLegacyMessage |
There was a problem hiding this comment.
Should this line be removed?
Verification