Update to cosmos-v0.37.0 by yun-yeo · Pull Request #231 · terra-money/classic-core

yun-yeo · 2019-09-03T05:38:03Z

Summary of changes

Update cosmos-sdk v0.37.0

All REST responses now wrap the original resource/result. The response
will contain two fields: height and result.
REST end points, which are changed

/market/params => /market/parameters
/oracle/params => /oracle/parameters
/treasury/tax-rate => /treasury/tax_rate
/treasury/tax-rate/{epoch} => /treasury/tax_rate/{epoch}
/treasury/tax-cap => /treasury/tax_cap
/treasury/tax-cap/{denom} => /treasury/tax_cap/{denom}
/treasury/reward-weight => /treasury/reward_weight
/treasury/reward-weight/{epoch} => /treasury/reward_weight/{epoch}
/treasury/tax-proceeds => /treasury/tax_proceeds
/treasury/tax-proceeds/{epoch} => /treasury/tax_proceeds/{epoch}
/treasury/seigniorage-proceeds => /treasury/seigniorage_proceeds
/treasury/seigniorage-proceeds/{epoch} => /treasury/seigniorage_proceeds/{epoch}
/treasury/current-epoch => /treasury/current_epoch
/treasury/params => /treasury/parameters

REST end points, which response object key is removed

/treasury/current_epoch
/treasury/seigniorage_proceeds/{epoch}
/treasury/seigniorage_proceeds
/treasury/tax_proceeds/{epoch}
/treasury/tax_proceeds
/treasury/reward_weight/{epoch}
/treasury/reward_weight
/treasury/tax_cap/{denom}
/treasury/tax_rate/{epoch}
/treasury/tax_rate
/oracle/denoms/actives
/oracle/denoms/{denom}/price
/oracle/denoms/{denom}/prevotes/{voter}
/oracle/denoms/{denom}/prevotes
/oracle/denoms/{denom}/votes/{voter}
/oracle/denoms/{denom}/votes

New REST endpoints

/supply/total
/supply/total/{denomination}
/market/last_day_issuance
/oracle/voters/{%s}/voting_info
/oracle/voting_infos
/treasury/historical_issuance/{epoch}

GradedVestingAccount is fully removed
LazyGradedVestingAccount's vesting_lazy_schedules is changed to vesting_schedules
Implement issue [FEATURE] Align the UX of fees(gas) and tax(stability) #200. The sender have to specify fees containing tax amount with following methods.
1. Use /bank/accounts/{address}/transfers without fees. terracli will compute tax amount and fill fees field containing both gas & tax fee.
2. Use /txs/estimate_fee to estimate fees of StdTX, and replace StdTx.Fee.Amount to estimated fee
3. Compute tax with /treasury/tax_rate & /treasury/tax_cap add computed tax with original gas fee

This resolves #219, resolves #175, resolves #195, resolves #197, resolves #200, resolves #209, and resolves #212

Report of required housekeeping

Github issue OR spec proposal link
Wrote tests
Updated relevant documentation (docs/)
Added a relevant changelog entry: clog add [section] [stanza] [message]

(FOR ADMIN) Before merging

Added appropriate labels to PR
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)
Confirm added tests are consistent with the intended behavior of changes
Ensure all tests pass

codecov · 2019-09-03T06:26:33Z

Codecov Report

Merging #231 into develop will increase coverage by 1.95%.
The diff coverage is 74.48%.

@@            Coverage Diff             @@
##           develop    #231      +/-   ##
==========================================
+ Coverage    65.74%   67.7%   +1.95%     
==========================================
  Files          105      68      -37     
  Lines         5958    2966    -2992     
==========================================
- Hits          3917    2008    -1909     
+ Misses        1799     849     -950     
+ Partials       242     109     -133

dokwon

Mainly stylistic feedback; did not review market / oracle given lots of the code will change.

dokwon · 2019-09-03T06:30:18Z

+	"github.com/terra-project/core/x/slashing"
+	"github.com/terra-project/core/x/staking"
 )



Comments for exported functions should start with the function name

dokwon · 2019-09-03T09:13:11Z

+- Test state that is needed by `tx` and `query` commands (`home`, `chain_id`, etc...) is stored on the `Fixtures` object. This makes constructing your new tests almost trivial.
+- Sometimes if you exit a test early there can be still running `terrad` and `terracli` processes that will interrupt subsequent runs. Still running `terracli` processes will block access to the keybase while still running `terrad` processes will block ports and prevent new tests from spinning up. You can ensure new tests spin up clean by running `pkill -9 terrad && pkill -9 terracli` before each test run.
+- Most `query` and `tx` commands take a variadic `flags` argument. This pattern allows for the creation of a general function which is easily modified by adding flags. See the `TxSend` function and its use for a good example.
+- `Tx*` functions follow a general pattern and return `(success bool, stdout string, stderr string)`. This allows for easy testing of multiple different flag configurations. See `TestGaiaCLICreateValidator` or `TestGaiaCLISubmitProposal` for a good example of the pattern.


TestGaiaCLICreateValidator --> TestTerraCLICreateValidator, TestGaiaCLISubmitProposal --> TestTerraCLISubmitProposal

dokwon · 2019-09-03T09:13:36Z

@@ -0,0 +1,51 @@
+# Gaia CLI Integration tests


Terra CLI Integration tests

dokwon · 2019-09-03T09:14:09Z

+	"github.com/cosmos/cosmos-sdk/x/staking"
+	authutils "github.com/terra-project/core/x/auth/client/utils"
+)
+


Long file; add short comment on how the test helpers are used

dokwon · 2019-09-03T09:29:41Z

-	rootCmd.AddCommand(version.VersionCmd)
+	rootCmd.AddCommand(genutilcli.InitCmd(ctx, cdc, app.ModuleBasics, app.DefaultNodeHome))
+	rootCmd.AddCommand(genutilcli.CollectGenTxsCmd(ctx, cdc, genaccounts.AppModuleBasic{}, app.DefaultNodeHome))
+	// terra does not need to support migrage


migrage --> migrate.

Why does terra not support this?

Recommend removing dead code altogether

dokwon · 2019-09-03T09:43:19Z

+
+	cmd.Flags().Float64(client.FlagGasAdjustment, client.DefaultGasAdjustment, "adjustment factor to be multiplied against the estimate returned by the tx simulation; if the gas limit is set manually this flag is ignored ")
+	cmd.Flags().String(client.FlagGasPrices, "", "Gas prices to determine the transaction fee (e.g. 10uluna)")
+	// cmd.MarkFlagRequired(client.FlagGasAdjustment)


Remove dead code

dokwon · 2019-09-03T09:49:09Z

+	"github.com/cosmos/cosmos-sdk/x/auth/types"
+	"github.com/cosmos/cosmos-sdk/x/simulation"
+)
+


Add comment for purpose of file

dokwon · 2019-09-03T09:49:25Z

+)
+
+// SimulateDeductFee
+func SimulateDeductFee(ak auth.AccountKeeper, supplyKeeper types.SupplyKeeper) simulation.Operation {


Strengthen comment to add detail on exported function

dokwon · 2019-09-03T09:55:11Z

+		return
+	}
+
+	// update luna issuance at last block of a day


Updatelastdayissuance --> updatePrevDayIssuance

dokwon · 2019-09-03T09:55:36Z

+	"github.com/terra-project/core/x/market/internal/types"
+)
+
+// market end block functionality


Endblocker is called at the end of every block.

dokwon

LGTM

yun-yeo requested a review from dokwon September 3, 2019 05:38

yun-yeo changed the title ~~update to cosmos-v0.37.0~~ Update to cosmos-v0.37.0 Sep 3, 2019

yun-yeo force-pushed the feature/update-cosmos-sdk-v0.37.0 branch from 530c0c9 to 4bab8f7 Compare September 3, 2019 05:48

yun-yeo added client rest / cli code improvements daemon daemon updates enhancement New feature or request must Mustfix for target release. labels Sep 3, 2019

yun-yeo self-assigned this Sep 3, 2019

yun-yeo force-pushed the feature/update-cosmos-sdk-v0.37.0 branch from 7dffed5 to 71e2f78 Compare September 3, 2019 08:53

dokwon suggested changes Sep 3, 2019

View reviewed changes

yun-yeo force-pushed the feature/update-cosmos-sdk-v0.37.0 branch 4 times, most recently from 063e008 to 9e1e284 Compare September 4, 2019 05:04

yun-yeo requested a review from dokwon September 4, 2019 05:39

update to cosmos-v0.37.0

23ea272

yun-yeo force-pushed the feature/update-cosmos-sdk-v0.37.0 branch from c6f4f8b to 23ea272 Compare September 4, 2019 06:23

dokwon approved these changes Sep 17, 2019

View reviewed changes

dokwon merged commit 2929e9f into develop Sep 17, 2019

dokwon deleted the feature/update-cosmos-sdk-v0.37.0 branch September 17, 2019 04:08

Conversation

yun-yeo commented Sep 3, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Update cosmos-sdk v0.37.0

Report of required housekeeping

(FOR ADMIN) Before merging

Uh oh!

codecov Bot commented Sep 3, 2019

Codecov Report

Uh oh!

dokwon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dokwon left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

yun-yeo commented Sep 3, 2019 •

edited

Loading