Skip to content
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

go/keymanager/api: Move key manager gas costs #5166

Merged
merged 1 commit into from
Feb 6, 2023

Conversation

peternose
Copy link
Contributor

@peternose peternose commented Feb 6, 2023

Moving key manager gas costs from registry to key manager.

@peternose peternose force-pushed the peternose/breaking/km-gas-costs branch 3 times, most recently from f755418 to 1179f0f Compare February 6, 2023 11:06
@peternose peternose marked this pull request as ready for review February 6, 2023 11:06
@peternose peternose added golang c:breaking/consensus Category: breaking consensus changes labels Feb 6, 2023
@codecov
Copy link

codecov bot commented Feb 6, 2023

Codecov Report

Merging #5166 (b4f54d5) into master (364c7f4) will increase coverage by 0.17%.
The diff coverage is 55.84%.

@@            Coverage Diff             @@
##           master    #5166      +/-   ##
==========================================
+ Coverage   66.73%   66.91%   +0.17%     
==========================================
  Files         497      499       +2     
  Lines       53300    53357      +57     
==========================================
+ Hits        35570    35704     +134     
+ Misses      13363    13291      -72     
+ Partials     4367     4362       -5     
Impacted Files Coverage Δ
go/keymanager/api/api.go 75.86% <0.00%> (-7.00%) ⬇️
go/registry/api/api.go 54.39% <ø> (+1.81%) ⬆️
go/consensus/tendermint/apps/keymanager/genesis.go 56.14% <33.33%> (-2.05%) ⬇️
...nsensus/tendermint/apps/keymanager/transactions.go 47.72% <40.00%> (-1.06%) ⬇️
...onsensus/tendermint/apps/keymanager/state/state.go 67.21% <46.66%> (-6.70%) ⬇️
...o/consensus/tendermint/apps/keymanager/messages.go 56.52% <56.52%> (ø)
go/keymanager/api/sanity_check.go 65.21% <65.21%> (ø)
go/oasis-node/cmd/genesis/genesis.go 54.36% <100.00%> (+0.26%) ⬆️
go/scheduler/api/sanity_check.go 71.42% <0.00%> (-9.53%) ⬇️
go/worker/compute/executor/committee/proposals.go 81.25% <0.00%> (-6.25%) ⬇️
... and 37 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

const (
// GasOpUpdateKeyManager is the gas operation identifier for key manager
// policy updates costs.
GasOpUpdateKeyManager transaction.Op = "update_keymanager"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be called GasOpUpdatePolicy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I also rename "update_keymanager" to "update_policy" and then remove "update_keymanager" in the fix genesis?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think that makes the most sense.

@peternose peternose force-pushed the peternose/breaking/km-gas-costs branch from 1179f0f to f445152 Compare February 6, 2023 14:00
@peternose peternose force-pushed the peternose/breaking/km-gas-costs branch from f445152 to b4f54d5 Compare February 6, 2023 17:07
@peternose peternose requested a review from kostko February 6, 2023 17:37
@peternose peternose merged commit fd955b7 into master Feb 6, 2023
@peternose peternose deleted the peternose/breaking/km-gas-costs branch February 6, 2023 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c:breaking/consensus Category: breaking consensus changes golang
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants