chore!: bump sdk fork to v1.17.0 #2278
Conversation
cmwaters
requested review from
rach-id,
evan-forbes and
rootulp
as code owners
There was a problem hiding this comment.
[not blocking]
This is super cool, but as long as the mapping PR is not merged, the QGB will be broken, therefore main will be broken.
Does it make sense to postpone bumping until we have that PR reviewed and ready to be merge? so that we don't have a list of commits which all contain a broken QGB in between
cmwaters
added
the
backport:v1.x
I was hoping I could kind of do these as two separate commits. Maybe it makes sense to merge the other one first |
Codecov Report
@@ Coverage Diff @@
## main #2278 +/- ##
==========================================
- Coverage 21.72% 21.72% -0.01%
==========================================
Files 129 129
Lines 14453 14450 -3
==========================================
- Hits 3140 3139 -1
+ Misses 11008 11006 -2
Partials 305 305
|
There was a problem hiding this comment.
I think this needs a make proto-gen per https://github.com/celestiaorg/celestia-app/actions/runs/5867854688/job/15909440521?pr=2278#step:3:83
| require.NoError(t, err) | ||
| staking.EndBlocker(ctx, input.StakingKeeper) | ||
| qgb.EndBlocker(ctx, *pk) | ||
| ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 10) | ||
|
|
||
| assert.Equal(t, currentAttestationNonce+1, pk.GetLatestAttestationNonce(ctx)) | ||
| // FIXME: this needs to change to 2 once we have a proper implementation of editing the EVM address |
There was a problem hiding this comment.
[question] not sure what you mean by "2"
There was a problem hiding this comment.
Ah it's currently 1 but we should be asserting that the latest nonce is 2
There was a problem hiding this comment.
I also should have @-ed myself but forgot to
Yes, as separate commit, but better they are sequential. Like we merge this one and the next commit on main would be the mapping commit. |
There was a problem hiding this comment.
this LGTM
fwiw I think its fine to merge to main and but then just not backport it yet to v1.x. This avoids using some temporaray feature branch, which is arguably just as bad as leaving main in a temporary unreleasable state (a property I'm not sure we need to preserve any more since we never want to cut releases directly from main)
cmwaters
removed
bot
Bumps to the latest version of the SDK. Note that this deactivates the use of the EVM address. To fulfill that functionality we will need to reintroduce a mapping in #2169
Bumps to the latest version of the SDK. Note that this deactivates the use of the EVM address. To fulfill that functionality we will need to reintroduce a mapping in #2169
Checklist