R4R: Replace GenTx with StdTx #2524

alessio · 2018-10-17T23:04:53Z

Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (docs/)
Added entries in PENDING.md with issue #
rereviewed Files changed in the github PR explorer

For Admin Use:

Added appropriate labels to PR (ex. wip, ready-for-review, docs)
Reviewers Assigned
Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

alessio · 2018-10-18T01:04:58Z

FTBFS due to:

go install -tags "netgo ledger" -ldflags "-X github.com/cosmos/cosmos-sdk/version.Version=0.24.2-886-g90013838" ./cmd/gaia/cmd/gaiad
# github.com/cosmos/cosmos-sdk/store
store/iavlstore.go:233:14: cannot use p (type []byte) as type *merkle.Proof in assignment
store/rootmultistore.go:303:12: cannot use buildMultiStoreProof(res.Proof, storeName, commitInfo.StoreInfos) (type []byte) as type *merkle.Proof in assignment
store/rootmultistore.go:303:38: cannot use res.Proof (type *merkle.Proof) as type []byte in argument to buildMultiStoreProof
store/rootmultistore.go:390:23: undefined: merkle.Hasher
Makefile:75: recipe for target 'install' failed
make: *** [install] Error 2
Exited with code 2

codecov · 2018-10-18T05:03:56Z

Codecov Report

Merging #2524 into develop will decrease coverage by 0.03%.
The diff coverage is 43.34%.

@@             Coverage Diff             @@
##           develop    #2524      +/-   ##
===========================================
- Coverage    57.88%   57.84%   -0.04%     
===========================================
  Files          140      141       +1     
  Lines         8119     8192      +73     
===========================================
+ Hits          4700     4739      +39     
- Misses        3122     3149      +27     
- Partials       297      304       +7

cwgoes

Thanks @alessio - see comments

client/utils/utils.go

cmd/gaia/app/app.go

cwgoes · 2018-10-18T05:13:43Z

cmd/gaia/app/genesis.go


 		// create the genesis account, give'm few steaks and a buncha token with there name
-		genaccs[i] = genesisAccountFromGenTx(genTx)
+		genaccs[i] = genesisAccountFromMsgCreateValidator(msg, freeFermionsAcc)


I guess this part is pretty Gaia-specific (as is needing to update the stake data).

But also, this won't actually be in Gaia, since we'll have an account set unrelated to the validator transactions. This needs to move to a special testnet command I think.

I think the approach that Gaia will use (separate account set) is likely to be common - and we might want to allow delegation transactions at genesis too.

Maybe this can be split into a separate PR. Also cc @zmanian - what do we need, if anything, for GoS genesis setup?

I guess we can use this for now and just strip the accounts generated here and use the fundraiser distribution instead, copy/paste job.

cmd/gaia/app/genesis.go

x/stake/client/cli/tx.go

cmd/gaia/app/app.go

cmd/gaia/app/genesis.go

cwgoes · 2018-10-18T23:08:30Z

@alessio Looks like the linter & module simulations are failing?

alessio · 2018-10-18T23:43:10Z

@cwgoes integration tests failed too - I'm on it

alessio · 2018-10-19T00:39:30Z

Good to go @cwgoes

client/lcd/lcd_test.go

sunnya97 · 2018-10-18T20:56:11Z

cmd/gaia/app/genesis.go

+	}
+
+	sort.Strings(addresses)
+	persistentPeers = strings.Join(addresses, ",")


I really don't think we should be using the Memo field of the StdTxs to propogate persistent peer ids.... What if someone wants to put something different in the Memo field?

Also seems to be conflating config stuff with genesis file stuff.

@cwgoes @jaekwon @alessio

I'm happy to get rid of it

jaekwon · 2018-10-19T01:30:13Z

cmd/gaia/app/genesis.go

-	if err != nil {
-		return
+	// skip stakeData validation as genesis is created from txs
+	if len(genesisState.Txs) > 0 {


Pretty sure we want to validate regardless for consistency of the genesisState.* information. For example, this validates parameters as well. And in the future I think we'll want to support loading genesis w/ stake/slashing/etc data as well as genesis txs... such as when recovering from a halted chain, though more changes will be necessary then.

alessio · 2018-10-19T03:49:57Z

I've reintroduced gaiad gentx for users convenience. I'll update docs accordingly ASAP

Finalize gaiacli tx create-validator --genesis-format

…; OverwriteKeys -> OverwriteKey

…-rebased

cwgoes

Tested ACK - thanks @alessio

We could use a little more cleanup on CLI output, but that can happen later.

alessio requested review from cwgoes, ebuchman and rigelrozanski as code owners October 17, 2018 23:04

cwgoes suggested changes Oct 18, 2018

View reviewed changes

alessio changed the title ~~Replace GenTx with StdTx~~ R4R: Replace GenTx with StdTx Oct 18, 2018

alessio added the ready-for-review label Oct 18, 2018

melekes reviewed Oct 18, 2018

View reviewed changes

cmd/gaia/app/app.go Show resolved Hide resolved

cmd/gaia/app/app.go Outdated Show resolved Hide resolved

cmd/gaia/app/genesis.go Outdated Show resolved Hide resolved

alessio mentioned this pull request Oct 18, 2018

WIP: Replace GenesisTx with StdTx #2422

Closed

5 tasks

alessio requested review from jaekwon, sunnya97 and ValarDragon October 19, 2018 00:09

jaekwon reviewed Oct 19, 2018

View reviewed changes

client/lcd/lcd_test.go Outdated Show resolved Hide resolved

sunnya97 reviewed Oct 19, 2018

View reviewed changes

jaekwon reviewed Oct 19, 2018

View reviewed changes

alessio requested a review from zramsay as a code owner October 19, 2018 04:24

Alessio Treglia added 9 commits October 18, 2018 21:26

Add arg to PrintUnsignedStdTx() to actually operate in offline mode

38d0336

Finalize gaiacli tx create-validator --genesis-format

New func processStdTxs()

36845cf

New function to create genesis account from MsgCreateValidator

ffb5e46

Remove gen-tx command

7060839

Update gaia to work with auth.StdTx

d365b11

Temporarily disable dead code

cf63a53

Fix build failures

73138e5

Remove dead code

622acc1

Keep on cleaning up

9879a17

Alessio Treglia and others added 14 commits October 18, 2018 21:26

Call app.slashingKeeper.AddValidators() even if len(txs) == 0

9c5d1c4

Refactoring, introduce gaiad init --skip-genesis, code cleanup

a33a880

CLI refactoring

1db33c3

Fix linter warnings, integration tests

ee36213

Update PENDING.md

898883f

Run make format

731d49a

Drop unnecessary workaround to make lcd_tests pass

f2878af

Ensure GenTxsDir is not empty

e28be2f

Reintroduce gentx

75b13d0

Fix FTBFS

d6d4b18

Simple name changes, GenesisState.Txs -> .GenTxs; OWK -> OverwriteKey…

3df8782

…; OverwriteKeys -> OverwriteKey

Make linter happy

cbe5c00

Update changelog

0641031

Update PENDING.md, docs

256919f

alessio force-pushed the alessio/1890-replace-genesistx-with-stdtx-rebased branch from 6b56153 to 256919f Compare October 19, 2018 04:27

cwgoes added 5 commits October 19, 2018 19:08

Merge branch 'develop' into alessio/1890-replace-genesistx-with-stdtx…

b2bb62e

…-rebased

'make format'

0164024

Start simulation at height 1, like Tendermint

7ab120f

Ensure we never fetch from height 0

7a9c9d9

Fix it the right way

aed1fba

cwgoes approved these changes Oct 19, 2018

View reviewed changes

cwgoes merged commit 593921d into develop Oct 19, 2018

cwgoes deleted the alessio/1890-replace-genesistx-with-stdtx-rebased branch October 19, 2018 18:00

This was referenced Oct 19, 2018

Start chain with initial state + sequence of transactions #1890

Closed

R4R: Minting #2527

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

R4R: Replace GenTx with StdTx #2524

R4R: Replace GenTx with StdTx #2524

alessio commented Oct 17, 2018 •

edited

Loading

alessio commented Oct 18, 2018

codecov bot commented Oct 18, 2018 •

edited

Loading

cwgoes left a comment

cwgoes Oct 18, 2018

cwgoes Oct 18, 2018

cwgoes Oct 18, 2018

cwgoes Oct 18, 2018

jaekwon Oct 19, 2018

cwgoes commented Oct 18, 2018

alessio commented Oct 18, 2018

alessio commented Oct 19, 2018

sunnya97 Oct 18, 2018

alessio Oct 19, 2018

jaekwon Oct 19, 2018

alessio commented Oct 19, 2018

cwgoes left a comment

R4R: Replace GenTx with StdTx #2524

R4R: Replace GenTx with StdTx #2524

Conversation

alessio commented Oct 17, 2018 • edited Loading

alessio commented Oct 18, 2018

codecov bot commented Oct 18, 2018 • edited Loading

Codecov Report

cwgoes 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

cwgoes commented Oct 18, 2018

alessio commented Oct 18, 2018

alessio commented Oct 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alessio commented Oct 19, 2018

cwgoes left a comment

Choose a reason for hiding this comment

alessio commented Oct 17, 2018 •

edited

Loading

codecov bot commented Oct 18, 2018 •

edited

Loading