Update to cosmos-v0.37.0 #231

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

app/export.go

-	tmtypes "github.com/tendermint/tendermint/types"
+
+	"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

cli_test/README.md

+- 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

cli_test/README.md

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


Terra CLI Integration tests

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

cli_test/test_helpers.go

+	"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

cmd/terrad/main.go

-	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

x/auth/client/cli/estimate_fee.go

+
+	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

x/auth/simulation/fake.go

+	"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

x/auth/simulation/fake.go

+)
+
+// 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

x/market/abci.go

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


Updatelastdayissuance --> updatePrevDayIssuance

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

x/market/abci.go

+	"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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update to cosmos-v0.37.0 #231

Update to cosmos-v0.37.0 #231

yun-yeo commented Sep 3, 2019 •

edited

Loading

codecov bot commented Sep 3, 2019

dokwon left a comment

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon Sep 3, 2019

dokwon left a comment

Update to cosmos-v0.37.0 #231

Update to cosmos-v0.37.0 #231

Conversation

yun-yeo commented Sep 3, 2019 • edited Loading

Summary of changes

Update cosmos-sdk v0.37.0

Report of required housekeeping

(FOR ADMIN) Before merging

codecov bot commented Sep 3, 2019

Codecov Report

dokwon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dokwon left a comment

Choose a reason for hiding this comment

yun-yeo commented Sep 3, 2019 •

edited

Loading