Upgrade SDK to 0.47 #1465
Upgrade SDK to 0.47 #1465
Conversation
itsdevbear
requested review from
MalteHerrmann and
4rgon4ut
and removed request for
a team
|
|
There was a problem hiding this comment.
This is quite a lot to digest and there seems to be a mixup with the SDK version, which was the basis for quite a few changes?
Also, I see quite a lot of stuff commented out and several things, that I could not really put into context as to how they are related to preparing the SDK update for v0.47.
Before I continue reviewing can you check the version issue?
|
@MalteHerrmann its a heavy WIP, commenting stuff out to get it compiling and then going to slowly start bringing stuff back :) If you look in |
@itsdevbear I see, thanks for putting in the work. :) Was only reviewing because you explicitly asked for a review a couple of hours ago. On a different note, I would still suggest to maybe rework the description you have added, because it says e.g. that #1460 breaks sdk v0.47, which is not true. |
|
Oh I think it auto added a review, but appreciate any help / time you have, since this is a bit of an ugly task haha. Notably if you're able to assist with the integration tests, Nix hates M1 ARM Macs and I haven't been able to get them running (we moved our @berachain repo's to full go testing no python or nix). #1460 was going to be broken, the 46.5 change has been backported into 0.47.x now, gotta just make the update. |
|
@MalteHerrmann Once we get a proper 0.47-alpha release cut it should clean up the dep mess. Should be soon? @tac0turtle :P |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Sets gas limit to max uint32 to not error with javascript dev tooling | ||
| // This -1 value indicating no block gas limit is set to max uint64 with geth hexutils | ||
| // which errors certain javascript dev tooling which only supports up to 53 bits | ||
| return int64(^uint32(0)), nil |
Check failure
Code scanning / gosec
Potential integer overflow by integer type conversion
|
Realised I had forgot to override the default mempool behaviour here. For those following along, I added a really basic mempool (with hella bugs) to get us going here. Next step will be to migrate a lot of the ante handler / feemarket into a mempool impl. |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1465 +/- ##
==========================================
- Coverage 68.70% 68.28% -0.42%
==========================================
Files 103 104 +1
Lines 9937 9980 +43
==========================================
- Hits 6827 6815 -12
- Misses 2730 2787 +57
+ Partials 380 378 -2
|
itsdevbear
requested review from
MalteHerrmann
and removed request for
4rgon4ut
|
Still need to add the proper mempool that can handle eth txn, but would love a once over @MalteHerrmann |
This reverts commit 00d6565.
|
I guess no need to do all these features in a single PR, we can upgrade dependency first, fix minimal breaking apis, then do the features one by one? |
|
This is pretty much the minimal. 0.47 is a huge upgrade. |
|
@itsdevbear FYI, this PR doesn't address #1231 because PostHandlers are only executed on a successful tx |
Closes: #1316, #1461
There are a set of tasks we probably want to hit simulateously to this upgrade.
Description
WIP
For contributor use:
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerFor admin use:
WIP,R4R,docs, etc)