-
Notifications
You must be signed in to change notification settings - Fork 113
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
rm template_id fields #840
Conversation
WalkthroughWalkthroughThe changes in this pull request focus on restructuring the action management system by removing certain template identifiers and renaming fields to enhance clarity. Key modifications include the removal of Changes
Possibly related PRs
Suggested labels
Tip OpenAI O1 model for chat
Recent review detailsConfiguration used: .coderabbit.yaml Files ignored due to path filters (80)
Files selected for processing (1)
Files skipped from review due to trivial changes (1)
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
Let's also regenerate types in |
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: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (53)
api/warden/warden/v1beta3/events.pulsar.go
is excluded by!api/**
api/warden/warden/v1beta3/key.pulsar.go
is excluded by!api/**
api/warden/warden/v1beta3/space.pulsar.go
is excluded by!api/**
api/warden/warden/v1beta3/tx.pulsar.go
is excluded by!api/**
tests/testdata/snapshot-base/config/genesis.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-base/config/gentx/gentx-01e813234f9a16dfeed98b1c8df08ca4354ceaf5.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-base/config/gentx/gentx-85d1e2d789cdc74d572a48b6de76477d9d1d79c9.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-base/config/node_key.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-base/config/priv_validator_key.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-base/keyring-test/0c47f8d9d3647451f3afdc04d21d74a41f3bb505.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-base/keyring-test/alice.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-base/keyring-test/b0b3c0320eee612d53ee503b33d8aa26deaa7423.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/config/genesis.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/config/gentx/gentx-365bc73652207de103158c338b33962456430b20.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/config/gentx/gentx-fa2b60327a87cbf7e17430f62307b66404a431d6.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/config/node_key.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/config/priv_validator_key.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/25f15d556cc34510107fe3bc0116bcdff066affd.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/42bce85c5294ebed9d3ad6910b2fbd74eb237b73.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/a11562a90ba0612c68e427b64809ed4b50431764.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/a5180d2e8edfa2f50ba335a86f342b1b28525161.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/bob.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/d0e1f31bd583c4ef018dc24a9ca69cdb58a21e0b.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/db6dc8882333ce2b44c4b7ab57a40d0ec174cb34.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/val.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-keychain/keyring-test/writer.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/config/genesis.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/config/gentx/gentx-0af5fa98eea8f957a7ee0c30d6a1333e40382b65.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/config/gentx/gentx-bf961c82820f8b45262d42d29d5b5eed6aa82998.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/config/node_key.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/config/priv_validator_key.json
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/0d16cb2369afa892c981bda57d01773fa17e15eb.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/152460c2a0b87f0bdb86b26d43b6b6e4303ca2fd.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/4321b27aa17effe4688897cd2d7eca0866988f9b.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/57ff6c09751305d95096e7b69bf62784450b00b1.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/71888ecb7124b1258a0cd9492cf5ddbd5c875416.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/772e4eb79bdcd57db625471ae1deff5c0af164dd.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/7e2d7a6a9fba5f6d42a6ce681fddc97e9f36bd04.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/806b596dcce7d6ec57c299900b1a1aa75462f536.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/832cfcbe7f1b48c2cdfdf1cfa457315912258070.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/93258175732613f8e73b3f439638a038de8a64d7.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/ac147d880a5b72f0c84426791a34a84887d778b6.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/alice.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/bob.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/charlie.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/dave.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/e1dba28c2ce851da31adcfdcb212059d1d49ffab.address
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/erin.info
is excluded by!tests/testdata/**
tests/testdata/snapshot-many-users/keyring-test/frank.info
is excluded by!tests/testdata/**
warden/x/warden/types/v1beta3/events.pb.go
is excluded by!**/*.pb.go
warden/x/warden/types/v1beta3/key.pb.go
is excluded by!**/*.pb.go
warden/x/warden/types/v1beta3/space.pb.go
is excluded by!**/*.pb.go
warden/x/warden/types/v1beta3/tx.pb.go
is excluded by!**/*.pb.go
Files selected for processing (14)
- CHANGELOG.md (1 hunks)
- cmd/wardend/cmd/gen-spaces.go (1 hunks)
- proto/warden/warden/v1beta3/events.proto (3 hunks)
- proto/warden/warden/v1beta3/key.proto (2 hunks)
- proto/warden/warden/v1beta3/space.proto (5 hunks)
- proto/warden/warden/v1beta3/tx.proto (3 hunks)
- tests/cases/keychain_writers.go (1 hunks)
- warden/app/wasm-interop/custom_msg.go (3 hunks)
- warden/x/warden/keeper/key_requests.go (2 hunks)
- warden/x/warden/keeper/msg_server_fulfil_key_request.go (2 hunks)
- warden/x/warden/keeper/msg_server_new_key_request.go (2 hunks)
- warden/x/warden/keeper/msg_server_new_space.go (2 hunks)
- warden/x/warden/keeper/msg_server_update_key.go (0 hunks)
- warden/x/warden/keeper/msg_server_update_space.go (1 hunks)
Files not reviewed due to no reviewable changes (1)
- warden/x/warden/keeper/msg_server_update_key.go
Additional context used
Path-based instructions (9)
warden/x/warden/keeper/msg_server_new_space.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.cmd/wardend/cmd/gen-spaces.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_new_key_request.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/key_requests.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_fulfil_key_request.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/x/warden/keeper/msg_server_update_space.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.warden/app/wasm-interop/custom_msg.go (1)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/cases/keychain_writers.go (2)
Pattern
**/*.go
: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*
: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"CHANGELOG.md (1)
Pattern
**/*.md
: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (29)
warden/x/warden/keeper/msg_server_new_space.go (2)
14-18
: LGTM!The changes to the
Space
struct initialization are consistent with the PR objectives and provide more granular control over approval and rejection processes for the space's administrative and signing templates.
37-43
: Looks good!The updates to the
EventCreateSpace
event are consistent with the modifications made to theSpace
struct initialization and ensure that the emitted event reflects the current state of theSpace
object accurately.cmd/wardend/cmd/gen-spaces.go (1)
34-41
: LGTM!The changes to the
Space
struct align with the PR objective of removing thetemplate_id
field and enhance the granularity of template management. The new fields for approval and rejection processes are correctly initialized to 0.The code changes conform to the Uber Golang style guide.
warden/x/warden/keeper/msg_server_new_key_request.go (2)
48-49
: LGTM!The change from a single
TemplateId
to separateApproveTemplateId
andRejectTemplateId
fields improves the flexibility of the key request process by allowing distinct templates for approval and rejection scenarios. This aligns well with the PR objective of removing thetemplate_id
field.
60-66
: Looks good!Updating the emitted event to include the
ApproveTemplateId
andRejectTemplateId
fields instead ofTemplateId
ensures that the event accurately reflects the changes made to theKeyRequest
struct. This maintains consistency and correctness in the event emission logic.warden/x/warden/keeper/key_requests.go (1)
21-26
: LGTM!The changes to the
Key
struct initialization are consistent with the list of alterations provided in the AI summary. The addition of theApproveTemplateId
andRejectTemplateId
fields enhances the functionality of theKey
object by allowing it to store more information related to the approval and rejection templates. This potentially improves the handling of key requests by supporting more complex workflows regarding key approval and rejection processes.warden/x/warden/keeper/msg_server_fulfil_key_request.go (1)
45-50
: LGTM!The addition of the
ApproveTemplateId
andRejectTemplateId
fields to theEventNewKey
event enhances the event's data structure by including additional identifiers related to key approval and rejection templates. This change may be relevant for tracking or processing key requests more effectively.The code segment is correctly accessing these fields from the
key
object.warden/x/warden/keeper/msg_server_update_space.go (1)
58-62
: LGTM!The event emission has been correctly updated to include the new fields
ApproveAdminTemplateId
,RejectAdminTemplateId
,ApproveSignTemplateId
, andRejectSignTemplateId
from thespace
object. The changes are consistent with the AI-generated summary.proto/warden/warden/v1beta3/space.proto (5)
18-18
: LGTM!The change in the
nonce
field index from 7 to 4 is a minor improvement to the field ordering and does not affect the functionality.
30-30
: LGTM!The update to the
approve_admin_template_id
field index from 8 to 5 is necessary to maintain the correct field ordering after the removal of theadmin_template_id
field. It does not affect the functionality.
42-42
: LGTM!The update to the
reject_admin_template_id
field index from 9 to 6 is necessary to maintain the correct field ordering after the removal of theadmin_template_id
field. It does not affect the functionality.
54-54
: LGTM!The update to the
approve_sign_template_id
field index from 10 to 7 is necessary to maintain the correct field ordering after the removal of thesign_template_id
field. It does not affect the functionality.
66-66
: LGTM!The update to the
reject_sign_template_id
field index from 11 to 8 is necessary to maintain the correct field ordering after the removal of thesign_template_id
field. It does not affect the functionality.warden/app/wasm-interop/custom_msg.go (2)
24-29
: LGTM!The changes to the
NewKeyRequest
struct align with the PR objective of removing thetemplate_id
field. The introduction ofApproveTemplateId
andRejectTemplateId
fields enhances the functionality by allowing more nuanced control over the approval process. This change reflects a more complex interaction model and provides greater flexibility in template management.
60-65
: Looks good!The updates to the
MsgNewKeyRequest
message construction in thehandleNewKeyRequest
function are consistent with the changes made to theNewKeyRequest
struct. The removal of theTemplateId
field and the addition ofApproveTemplateId
andRejectTemplateId
fields ensure that the new fields are correctly propagated to the message. The changes are implemented accurately and maintain the integrity of the key request handling process.proto/warden/warden/v1beta3/key.proto (2)
62-63
: LGTM!Adding the
reject_template_id
field enhances functionality by allowing a separate template for the rejection process. The field number and positioning are appropriate.
107-107
: LGTM!Renaming
template_id
toapprove_template_id
improves clarity and maintains consistency with the change in theKeyRequest
message.tests/cases/keychain_writers.go (2)
43-43
: LGTM!The removal of the
--template-id
parameter aligns with the PR objective and streamlines the key request creation process. The test logic remains valid.
48-48
: Looks good!The removal of the
--template-id
parameter is consistent with the previous change and the PR objective. The test remains effective in verifying the key request creation with a different fee amount.proto/warden/warden/v1beta3/events.proto (5)
22-32
: LGTM!The changes to the
EventCreateSpace
message improve clarity by explicitly specifying the purpose of each template ID. The new field names are consistent with the PR objective of removing the generictemplate_id
field and providing more specific names.
41-50
: LGTM!The changes to the
EventUpdateSpace
message improve clarity by explicitly specifying the purpose of each template ID. The removal of the genericadmin_template_id
andsign_template_id
fields and the introduction of more specific approval and rejection template IDs are consistent with the PR objective.
88-98
: LGTM!The changes to the
EventNewKeyRequest
message improve clarity by explicitly specifying the purpose of each template ID. The replacement of the generictemplate_id
field withapprove_template_id
andreject_template_id
is consistent with the PR objective of providing more specific names.
115-119
: LGTM!The changes to the
EventNewKey
message improve clarity by explicitly specifying the purpose of each template ID. The replacement of the generictemplate_id
field withapprove_template_id
andreject_template_id
is consistent with the PR objective of providing more specific names.
Line range hint
1-1000
: Verify the presence ofreject_template_id
inEventUpdateKey
.The change to
approve_template_id
in theEventUpdateKey
message is consistent with the PR objective. However, the presence ofreject_template_id
without any annotation suggests that it may have been overlooked or is an inconsistency in the changes.Please ensure that the presence of
reject_template_id
is intentional and consistent with the changes made to other messages. If it is not required, consider removing it for consistency.proto/warden/warden/v1beta3/tx.proto (4)
92-96
: LGTM!The changes to
MsgNewSpace
introduce a more explicit approval/rejection mechanism for admin and sign templates, enhancing the clarity and functionality of the message structure. The field renumbering is consistent with the new approval/rejection fields.
147-151
: LGTM!The changes to
MsgUpdateSpace
are consistent with the updates made toMsgNewSpace
. The removal of the old template ID fields and the addition of the new approval/rejection fields suggest a shift towards a more explicit approval/rejection mechanism for templates. The field renumbering is consistent with the new approval/rejection fields, enhancing the clarity and functionality of the message structure.
194-202
: LGTM!The changes to
MsgNewKeyRequest
introduce a more explicit approval/rejection mechanism for templates, consistent with the updates made toMsgNewSpace
andMsgUpdateSpace
. The field renumbering is consistent with the new approval/rejection fields and themax_keychain_fees
field, enhancing the clarity and functionality of the message structure.
Line range hint
229-233
: LGTM!The changes to
MsgUpdateKey
are consistent with the updates made to other messages. The removal of thetemplate_id
field and the retention of the approval/rejection fields suggest a focus on managing key updates through explicit approval processes, enhancing the clarity and functionality of the message structure.CHANGELOG.md (1)
66-66
: LGTM!The changelog entry accurately captures the removal of template ID fields from the
Key
andSpace
components, which aligns with the stated PR objective. The entry is free of any spelling or grammatical errors.
// ID of the Template that the resulting Key will use with approve. | ||
uint64 approve_template_id = 8; |
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.
Breaking change: Handle the field number update.
Renaming template_id
to approve_template_id
improves clarity. However, updating the field number from 8 to 9 is a breaking change. Ensure that all code referencing this field is updated to use the new field number.
|
||
// ID of the Template that will need to be satisfied for rejecting using this key to sign | ||
// data. | ||
// If this is not set, the key will use the reject signing Template of the Space. | ||
uint64 reject_template_id = 8; | ||
uint64 reject_template_id = 7; |
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.
Breaking change: Handle the field number update.
Reordering the reject_template_id
field to align with the KeyRequest
message is a good change. However, updating the field number from 7 to 6 is a breaking change. Ensure that all code referencing this field is updated to use the new field number.
Let's also increment |
Cleanup missed template_id field from Key, Space and corresponding messages
Summary by CodeRabbit
New Features
@wardenprotocol/wardenjs
package to version 0.0.10.Bug Fixes
Documentation