-
Notifications
You must be signed in to change notification settings - Fork 108
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
fix(zetacore): bring back reserved fields as deprecated #3297
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces modifications to cross-chain transaction tracking mechanisms, focusing on enhancing proof and transaction metadata handling. The changes primarily involve updating protocol buffer message definitions across multiple files, adding new fields for proof tracking, and deprecating certain existing fields. The modifications aim to improve the robustness of transaction verification and indexing processes. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
14567965 | Triggered | Generic Password | c0c8076 | cmd/zetaclientd/start.go | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
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.
lgtm ,
Is there any way to test this before merging ?
We can maybe try putting this binary on a rpc node , and they trying to run the query and see if it works ?
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/spec/crosschain/messages.md (1)
15-17
: Fix indentation: Replace tabs with spaces.The message definitions use hard tabs. For better markdown compatibility and consistency, replace tabs with spaces.
Apply this formatting change:
- pkg.proofs.Proof proof = 5; - string block_hash = 6; - int64 tx_index = 7; + pkg.proofs.Proof proof = 5; + string block_hash = 6; + int64 tx_index = 7;Also applies to: 31-33
🧰 Tools
🪛 Markdownlint (0.37.0)
15-15: Column: 1
Hard tabs(MD010, no-hard-tabs)
16-16: Column: 1
Hard tabs(MD010, no-hard-tabs)
17-17: Column: 1
Hard tabs(MD010, no-hard-tabs)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
typescript/zetachain/zetacore/crosschain/outbound_tracker_pb.d.ts
is excluded by!**/*_pb.d.ts
typescript/zetachain/zetacore/crosschain/tx_pb.d.ts
is excluded by!**/*_pb.d.ts
x/crosschain/types/outbound_tracker.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
x/crosschain/types/tx.pb.go
is excluded by!**/*.pb.go
,!**/*.pb.go
📒 Files selected for processing (4)
docs/openapi/openapi.swagger.yaml
(1 hunks)docs/spec/crosschain/messages.md
(2 hunks)proto/zetachain/zetacore/crosschain/outbound_tracker.proto
(1 hunks)proto/zetachain/zetacore/crosschain/tx.proto
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
proto/zetachain/zetacore/crosschain/outbound_tracker.proto (1)
Pattern **/*.proto
: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.
proto/zetachain/zetacore/crosschain/tx.proto (1)
Pattern **/*.proto
: Review the Protobuf definitions, point out issues relative to compatibility, and expressiveness.
🪛 buf (1.47.2)
proto/zetachain/zetacore/crosschain/outbound_tracker.proto
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
🪛 Markdownlint (0.37.0)
docs/spec/crosschain/messages.md
15-15: Column: 1
Hard tabs
(MD010, no-hard-tabs)
16-16: Column: 1
Hard tabs
(MD010, no-hard-tabs)
17-17: Column: 1
Hard tabs
(MD010, no-hard-tabs)
31-31: Column: 1
Hard tabs
(MD010, no-hard-tabs)
32-32: Column: 1
Hard tabs
(MD010, no-hard-tabs)
33-33: Column: 1
Hard tabs
(MD010, no-hard-tabs)
🔇 Additional comments (5)
proto/zetachain/zetacore/crosschain/outbound_tracker.proto (2)
11-15
: LGTM: Proper deprecation of the proved
field.
The field is correctly marked as deprecated and excluded from JSON/YAML serialization, which aligns with the PR objective of maintaining proto decoder compatibility while preventing active usage.
4-4
:
Fix the import path for gogoproto.
The import "gogoproto/gogo.proto" needs to be properly configured in your build system. Ensure the proto path includes the gogoproto package.
🧰 Tools
🪛 buf (1.47.2)
4-4: import "gogoproto/gogo.proto": file does not exist
(COMPILE)
proto/zetachain/zetacore/crosschain/tx.proto (2)
69-83
: LGTM: Consistent deprecation pattern for proof-related fields.
The fields are properly marked as deprecated and excluded from serialization, maintaining backward compatibility while preventing active usage.
109-123
: LGTM: Consistent implementation with MsgAddInboundTracker.
The deprecation pattern matches the inbound tracker implementation, ensuring a uniform approach across the codebase.
docs/spec/crosschain/messages.md (1)
15-17
: LGTM: Documentation accurately reflects proto changes.
The message definitions in the documentation correctly reflect the changes made to the protocol buffer files, maintaining consistency between implementation and documentation.
Also applies to: 31-33
🧰 Tools
🪛 Markdownlint (0.37.0)
15-15: Column: 1
Hard tabs
(MD010, no-hard-tabs)
16-16: Column: 1
Hard tabs
(MD010, no-hard-tabs)
17-17: Column: 1
Hard tabs
(MD010, no-hard-tabs)
@kingpinXD @fadeev @CharlieMc0 fixed. However, due to zetacored q tx 6C86B759289ACB1BE2EEC35619B7F9227EC4AF4AC235BC70F366C1DCDF76539A --node=https://zetachain-mainnet.g.allthatnode.com:443/archive/tendermint -o json | jq {
"height": "6057922",
"txhash": "6C86B759289ACB1BE2EEC35619B7F9227EC4AF4AC235BC70F366C1DCDF76539A",
"codespace": "",
"code": 0,
"data": "122B0A252F636F736D6F732E617574687A2E763162657461312E4D736745786563526573706F6E736512020A00",
"raw_log": "[{\"msg_index\":0,\"events\":[{\"type\":\"message\",\"attributes\":[{\"key\":\"action\",\"value\":\"/cosmos.authz.v1beta1.MsgExec\"},{\"key\":\"sender\",\"value\":\"zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k\"},{\"key\":\"module\",\"value\":\"authz\"}]}]}]",
"logs": [
{
"msg_index": 0,
"log": "",
"events": [
{
"type": "message",
"attributes": [
{
"key": "action",
"value": "/cosmos.authz.v1beta1.MsgExec"
},
{
"key": "sender",
"value": "zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k"
},
{
"key": "module",
"value": "authz"
}
]
}
]
}
],
"info": "",
"gas_wanted": "200000",
"gas_used": "104673",
"tx": {
"@type": "/cosmos.tx.v1beta1.Tx",
"body": {
"messages": [
{
"@type": "/cosmos.authz.v1beta1.MsgExec",
"grantee": "zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k",
"msgs": [
{
"@type": "/zetachain.zetacore.crosschain.MsgAddOutboundTracker",
"creator": "zeta19jr7nl82lrktge35f52x9g5y5prmvchmk40zhg",
"chain_id": "1",
"nonce": "46275",
"tx_hash": "0x878345cdd31740ce8c14c0638b66873911be39889afe654e3301dc47dcf1717b",
"proof": null,
"block_hash": "",
"tx_index": "-1"
}
]
}
],
"memo": "",
"timeout_height": "0",
"extension_options": [],
"non_critical_extension_options": []
},
"auth_info": {
"signer_infos": [
{
"public_key": {
"@type": "/ethermint.crypto.v1.ethsecp256k1.PubKey",
"key": "A7JcN5KwyoI7U+WkuklFZZE//e9GiWow5IuFRaMoTUlr"
},
"mode_info": {
"single": {
"mode": "SIGN_MODE_DIRECT"
}
},
"sequence": "255890"
}
],
"fee": {
"amount": [
{
"denom": "azeta",
"amount": "30000000000000"
}
],
"gas_limit": "200000",
"payer": "",
"granter": ""
},
"tip": null
},
"signatures": [
"qlf6++n0CyhLTULG358j1JEm7WCsRf0FRevDU5hwhEcpAZ+Z9occNa1ASGrBNVJvla30fqRc73LbjULTB43IggE="
]
},
"timestamp": "2024-12-07T14:56:47Z",
"events": [
{
"type": "coin_spent",
"attributes": [
{
"key": "spender",
"value": "zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k",
"index": true
},
{
"key": "amount",
"value": "30000000000000azeta",
"index": true
}
]
},
{
"type": "coin_received",
"attributes": [
{
"key": "receiver",
"value": "zeta17xpfvakm2amg962yls6f84z3kell8c5lxad43d",
"index": true
},
{
"key": "amount",
"value": "30000000000000azeta",
"index": true
}
]
},
{
"type": "transfer",
"attributes": [
{
"key": "recipient",
"value": "zeta17xpfvakm2amg962yls6f84z3kell8c5lxad43d",
"index": true
},
{
"key": "sender",
"value": "zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k",
"index": true
},
{
"key": "amount",
"value": "30000000000000azeta",
"index": true
}
]
},
{
"type": "message",
"attributes": [
{
"key": "sender",
"value": "zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k",
"index": true
}
]
},
{
"type": "tx",
"attributes": [
{
"key": "fee",
"value": "30000000000000azeta",
"index": true
},
{
"key": "fee_payer",
"value": "zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k",
"index": true
}
]
},
{
"type": "tx",
"attributes": [
{
"key": "acc_seq",
"value": "zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k/255890",
"index": true
}
]
},
{
"type": "tx",
"attributes": [
{
"key": "signature",
"value": "qlf6++n0CyhLTULG358j1JEm7WCsRf0FRevDU5hwhEcpAZ+Z9occNa1ASGrBNVJvla30fqRc73LbjULTB43IggE=",
"index": true
}
]
},
{
"type": "message",
"attributes": [
{
"key": "action",
"value": "/cosmos.authz.v1beta1.MsgExec",
"index": true
},
{
"key": "sender",
"value": "zeta15l7fehe8azkmj6979ln7e74dkvl0ypledqje7k",
"index": true
},
{
"key": "module",
"value": "authz",
"index": true
}
]
}
]
} |
Thanks , verfied that the tx can be quired with the new binary , and also that the tx cannot be queried with the older one . |
Cosmos SDK's proto decoder explicitly fails to decode fields marked as "reserved." This PR reinstates legacy fields as deprecated.
Deprecated fields should also be excluded from JSON and YAML encodersCloses #3280
Summary by CodeRabbit
New Features
proved
to the OpenAPI specifications.MsgAddOutboundTracker
andMsgAddInboundTracker
messages, including proof and indexing information.Bug Fixes
TxHash
,MsgAddOutboundTracker
, andMsgAddInboundTracker
messages to improve clarity and functionality.Documentation