-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: Reuse v6 starknet_getTransactionByHash handler for v7 #2486
Conversation
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.
We should also be handling the pending
block in v6
in the same way. As far as I know, this difference happened because the old code for v6
was copied in #2419 . You can use the same code from v7
and continue refactoring.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2486 +/- ##
==========================================
+ Coverage 74.21% 74.66% +0.44%
==========================================
Files 138 139 +1
Lines 16844 16726 -118
==========================================
- Hits 12501 12488 -13
+ Misses 3488 3404 -84
+ Partials 855 834 -21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Ah yes indeed, good catch! I updated v6 code, and I could then reuse v6 handler for v7. |
rpc/v7/transaction_test.go
Outdated
}) | ||
} | ||
|
||
// Convert a v6 transaction object to a v7 transaction object | ||
func AdaptV6TxToV7(t *testing.T, tx *rpcv6.Transaction) *rpc.Transaction { |
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 think we can un-export this
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.
Done!
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.
Transfer coins Ethereum
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.
Just need to un-export AdaptV6TxToV7
and then it looks good to me
rpc pkg cleanup according to issue #2437
for v6: Updated handler code to take into account pending block as well.
for v7: Reuse v6 handler for v7 as the logic is the same.
for v8: The logic is the same as v6/v7, but not exactly the returned
Transaction
type (more particularly,Resource
type insideTransaction
type). Therefore, we cannot refactor v8's types or logic