regeneration of app #127
Merged
regeneration of app #127
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ionDataPayForMessage
evan-forbes
force-pushed
the
evan/regenerate-app
branch
from
0a7a685 to
0e77e38
Compare
Codecov Report
@@ Coverage Diff @@
## master #127 +/- ##
==========================================
- Coverage 19.58% 19.49% -0.10%
==========================================
Files 15 14 -1
Lines 2783 2750 -33
==========================================
- Hits 545 536 -9
+ Misses 2211 2185 -26
- Partials 27 29 +2
Continue to review full report at Codecov.
|
evan-forbes
commented
Sep 30, 2021
Comment on lines
+90
to
+106
| // pfmURL is the URL expected for pfm. NOTE: this will be deleted when we upgrade from | ||
| // sdk v0.44.0 | ||
| var pfmURL = sdk.MsgTypeURL(&types.MsgWirePayForMessage{}) | ||
|
|
||
| func hasWirePayForMessage(tx sdk.Tx) bool { | ||
| for _, msg := range tx.GetMsgs() { | ||
| if msg.Type() == types.TypeMsgPayforMessage { | ||
| msgName := sdk.MsgTypeURL(msg) | ||
| if msgName == pfmURL { | ||
| return true | ||
| } | ||
| // note: this is what we will use in the future as proto.MessageName is | ||
| // deprecated | ||
| // svcMsg, ok := msg.(sdk.ServiceMsg) if !ok { | ||
| // continue | ||
| // } if svcMsg.SerivceMethod == types.TypeMsgPayforMessage { | ||
| // return true | ||
| // } |
There was a problem hiding this comment.
this will have to be replaced as soon as we use a newer version of the sdk (see comment).
| option go_package = "github.com/celestiaorg/celestia-core/abci/types"; | ||
| option go_package = "github.com/tendermint/tendermint/abci/types"; |
There was a problem hiding this comment.
this closes #145! 🙌 This is actually really nice, as we will be able to use the cli in scripts w/o getting messed up output. i.e
($celestia-appd tendermint show-node-id)| expected: []byte{0x5d, 0x43, 0xd7, 0x40, 0xe5, 0xe6, 0x5e, 0x2a, 0xb9, 0x10, 0x5c, 0xf9, 0x26, 0xf9, 0xf0, 0x1c, 0x3a, 0x11, 0x49, 0x1c, 0x71, 0x21, 0xdf, 0x46, 0xdd, 0x21, 0x94, 0x3f, 0xba, 0xb1, 0xcf, 0xd4}, | ||
| expected: []byte{0x1c, 0x57, 0x89, 0x2f, 0xbe, 0xbf, 0xa2, 0xa4, 0x4c, 0x41, 0x9e, 0x2d, 0x88, 0xd5, 0x87, 0xc0, 0xbd, 0x37, 0xc0, 0x85, 0xbd, 0x10, 0x3c, 0x36, 0xd9, 0xa2, 0x4d, 0x4e, 0x31, 0xa2, 0xf8, 0x4e}, |
There was a problem hiding this comment.
we updated to a new version of nmt, which includes the namespace in the hash, which changes this output
| expected: []byte(`{"fee":{"base_rate_max":"10000","tip_rate_max":"1000"},"message_namespace_id":"AQIDBAECAwQ=","message_share_commitment":"byozRVIrw5NF/rU1PPyq6BAo3g2ny3uLTiOFedtgSwo=","message_size":"4","nonce":"1"}`), | ||
| expected: []byte(`{"fee":{"base_rate_max":"10000","tip_rate_max":"1000"},"message_namespace_id":"AQIDBAECAwQ=","message_share_commitment":"Elh5P8yB1FeiPP0uWCkp67mqSsaVat6iwjH2vSMQJys=","message_size":"4","nonce":"1"}`), |
There was a problem hiding this comment.
same as above, new version of nmt causes the commitments to be different
adlerjohn
reviewed
Sep 30, 2021
Bidon15
reviewed
Sep 30, 2021
Co-authored-by: Nguyen Nhu Viet <[email protected]>
they're meant for scaffolding using starport, and if we end up using them, then we can add them back in. deleted in 0d0b88a thanks! |
Co-authored-by: Nguyen Nhu Viet <[email protected]>
13 tasks
Bidon15
previously approved these changes
Oct 4, 2021
Bidon15
previously approved these changes
Oct 4, 2021
|
Do you have any idea why some of the files changed pems to executable? |
adlerjohn
requested changes
Oct 6, 2021
…er-app into evan/regenerate-app
evan-forbes
dismissed stale reviews from liamsi, tzdybal, and Bidon15
via
0ebad95
I'm super sorry, I vaguely remember changing permissions on some files after generating them in starport because it wouldn't let me do anything with them, and I clearly did not change them back/appropriately. permissions we're changed back in bcabdfa thank you for catching that!! |
adlerjohn
approved these changes
Oct 7, 2021
rootulp
added a commit
to rootulp/celestia-app
that referenced
this pull request
Jul 14, 2022
cosmos/keyring hasn't pulled upstream changes from 99designs/keyring yet and the latest release of 99designs/keyring (1.2.1) appears to contain a fix for MacOS warnings: 99designs/keyring#107 Links - https://github.com/cosmos/keyring/tags - https://github.com/99designs/keyring/releases/tag/v1.2.1 `make test` passes after this change and it looks like other Cosmos chains don't need this replace: - https://github.com/osmosis-labs/osmosis/blob/main/go.mod#L274-L285= - https://github.com/CosmosContracts/juno/blob/main/go.mod#L178-L182= Does anyone know if it's still necessary? I couldn't find the justification in celestiaorg#127 which added it.
Merged
rootulp
added a commit
that referenced
this pull request
Aug 11, 2022
cosmos/keyring hasn't pulled upstream changes from 99designs/keyring yet and the latest release of 99designs/keyring (1.2.1) appears to contain a fix for MacOS warnings: 99designs/keyring#107 Links - https://github.com/cosmos/keyring/tags - https://github.com/99designs/keyring/releases/tag/v1.2.1 `make test` passes after this change and it looks like other Cosmos chains don't need this replace: - https://github.com/osmosis-labs/osmosis/blob/main/go.mod#L274-L285= - https://github.com/CosmosContracts/juno/blob/main/go.mod#L178-L182= Does anyone know if it's still necessary? I couldn't find the justification in #127 which added it.
0xchainlover
pushed a commit
to celestia-org/celestia-app
that referenced
this pull request
Aug 1, 2024
cosmos/keyring hasn't pulled upstream changes from 99designs/keyring yet and the latest release of 99designs/keyring (1.2.1) appears to contain a fix for MacOS warnings: 99designs/keyring#107 Links - https://github.com/cosmos/keyring/tags - https://github.com/99designs/keyring/releases/tag/v1.2.1 `make test` passes after this change and it looks like other Cosmos chains don't need this replace: - https://github.com/osmosis-labs/osmosis/blob/main/go.mod#L274-L285= - https://github.com/CosmosContracts/juno/blob/main/go.mod#L178-L182= Does anyone know if it's still necessary? I couldn't find the justification in celestiaorg/celestia-app#127 which added it.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR regenerates the app using the latest version of starport, our fork of the sdk based on v0.44.0, and our version of celestia-core that is based on v0.34.12 of tendermint.
I tried to limit the changes in this PR to strictly regenerating/upgrading and not refactoring, which we still need to do.
closes #114
blocked by #33/#34