-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat: update Connection
to support versioned transactions
#27068
Conversation
Codecov Report
@@ Coverage Diff @@
## master #27068 +/- ##
=========================================
- Coverage 76.7% 76.2% -0.5%
=========================================
Files 52 52
Lines 2651 2674 +23
Branches 364 372 +8
=========================================
+ Hits 2035 2040 +5
- Misses 483 495 +12
- Partials 133 139 +6 |
55f9541
to
5cb45da
Compare
5cb45da
to
03e8f69
Compare
5f92c3a
to
95b85e1
Compare
95b85e1
to
1f88fcf
Compare
There is a breaking change in this update so according to semver we should bump the major version of the package. Last time we did this was in e9b08b5 over a year ago. Please discuss if you have any concerns about this, thanks! |
1f88fcf
to
bca68f0
Compare
Connection
to support versioned transactions
Connection
to support versioned transactionsConnection
to support versioned transactions
There are a few strategies to roll a change to return types incrementally that we could use here. 1. Rename, deprecate, then removeWe could deliver
2. Sidecar methodsInstead of changing the old methods, create new ones: Either of these would let us continue to ship new unrelated features into |
bca68f0
to
25e172a
Compare
I opted for overloading the methods and only returning a versioned transaction type if
Yeah, that would be a headache. It'd be great to queue up have infra that allows us to ship to multiple major versions. Until then, it's not worth the risk and overhead of shipping a new major version right now |
Connection
to support versioned transactionsConnection
to support versioned transactions
25e172a
to
310e7a1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Phew! Thanks!
Didn't look at the code in detail, just approving the approach in principle. cc/ @jordansexton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the same concerns as @steveluscher around making this a breaking change at this time, but this approach looks solid!
…abs#27068) feat: update Connection to support versioned transactions
Problem
Methods like
Connection.getTransaction
andConnection.getBlock
support themaxSupportedTransactionVersion
config but the return type contains aMessage
class which doesn't support versioned messages.Summary of Changes
getTransaction
,getTransactions
, andgetBlock
RPC APIs without passing amaxSupportedTransactionVersion
in the configgetTransaction
,getParsedTransaction
,getBlock
, andgetTransactions
responsesThis change is not a breaking change because the responses for the modified
Connection
methods only include a versioned message when a user opts into receiving versioned transactions with themaxSupportedTransactionVersion
config.Fixes #