docs via example apps #1376

ebuchman · 2018-06-26T02:06:16Z

Implements #1330

codecov · 2018-06-26T02:34:27Z

Codecov Report

Merging #1376 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #1376   +/-   ##
========================================
  Coverage    62.16%   62.16%           
========================================
  Files          103      103           
  Lines         5838     5838           
========================================
  Hits          3629     3629           
  Misses        1999     1999           
  Partials       210      210

ebuchman · 2018-06-27T21:24:04Z

This is still a WIP but a lot of the content is in place in a really good way again and I think we could merge to develop and then pick up from there.

alexanderbez

This is great @ebuchman. It helped me a lot in terms of understanding the overall SDK structure. I left a few comments and questions 👍

There are still a handful of TODO's so I take it this is still a WIP?

alexanderbez · 2018-06-27T20:10:29Z

docs/core/app1.md

+ }
+```
+
+Note it is unforgiving - it panics on nil keys!


Is it desirable to panic on nil keys here? Or can we bubble an error upstream to handle gracefully...or panic?

#870 (comment)

alexanderbez · 2018-06-27T20:13:13Z

docs/core/app1.md

+
+We have only a single message type, so just one message-specific function to define, `handleMsgSend`.
+
+Note this handler has unfettered access to the store specified by the capability key `keyAcc`. So it must also define items in the store are encoded.


So it must also define items in the store are encoded. A bit confusing to me.

alexanderbez · 2018-06-27T20:14:47Z

docs/core/app1.md

+
+```go
+// Handle MsgSend.
+// NOTE: msg.From, msg.To, and msg.Amount were already validated


Do you think it might be worthwhile mentioning how and where validation occurs (i.e. What calls ValidateBasic?

alexanderbez · 2018-06-28T12:51:09Z

docs/core/app2.md

+	sdk.Msg
+
+    PubKey    crypto.PubKey
+	Signature crypto.Signature


Bad indentation here :)

alexanderbez · 2018-06-28T13:24:49Z

docs/core/examples/app2.go

+			}
+
+			// Return error result if msg Issuer is not equal to coin issuer
+			if !reflect.DeepEqual(metadata.Issuer, msg.Issuer) {


Not sure what the overhead of reflect is here, but do you think it'll be easier to understand and slightly more performant if we do: bytes.Equal(metadata.Issuer.Bytes(), msg.Issuer.Bytes())?

alexanderbez · 2018-06-28T13:42:12Z

docs/core/app3.md

+amount of coins in an account:
+
+```go
+acc := GetAccount(ctx, addr)


acc := accountMapper.GetAccount(ctx, addr)?

alexanderbez · 2018-06-28T13:50:31Z

docs/core/app3.md

+the store.
+
+Creating an AccountMapper is easy - we just need to specify a codec, a
+capability key, and a prototype of the object being encoded (TODO: change to


How do we anticipate the constructor changing?

instead of passing in a prototype, we pass in a constructor: #1379

alexanderbez · 2018-06-28T14:31:44Z

docs/core/app3.md

+The PubKey is required for signature verification, but it is only required in
+the StdSignature once. From that point on, it will be stored in the account.
+
+The fee is paid by the first address returned by `msg.GetSigners()` for the first `Msg`. 


I see this function, but in the actual Antehandler impl, I don't see this being called? It just uses the first sig it sees from the TX.

first sig in the tx must match first signer in the msgs

I meant I don't see FeePayer actually being used, but I don't think it's a big deal.

alexanderbez · 2018-06-28T14:41:32Z

docs/core/app3.md

+
+Earlier, we noted that `Mappers` abstactions over a KVStore that handles marshalling and unmarshalling a
+particular data type to and from the underlying store. We can build another
+abstraction on top of `Mappers` that we call `Keepers`, which expose only


👌 I really like this abstraction

alexanderbez · 2018-06-28T14:58:12Z

docs/core/examples/app3.go

+	accountMapper := auth.NewAccountMapper(cdc, keyAccount, &auth.BaseAccount{})
+	accountKeeper := bank.NewKeeper(accountMapper)
+	metadataMapper := NewApp3MetaDataMapper(keyMain)
+	feeKeeper := auth.NewFeeCollectionKeeper(cdc, keyFees)


The underlying implementation also performs serialization logic which makes it seem more of a mapper. Can Keepers also perform the same logic as Mappers? Seems like a bit of a blurry line.

left as a TODO for now to clarify the feeKeeper (not sure if we can remove it from the base AnteHandler, or else we need to say something like we'll explain it after since we introduce the AnteHandler first

Bucky aditya/docs core

alexanderbez

Changes look great! I think it's good step forward indeed.

ebuchman · 2018-06-29T06:00:41Z

Apps1-3 are basically complete.

Everything else still needs a bunch of work, but all the links work and every thing at least points in the direction of more information until we have time to fill things in and explain properly :)

We should spend some time on the UX before we provide full docs for the CLI. In the meantime we just point to the testnet tutorial.

We should keep working on the writing for app4-5 and the client (at least explain the LCD), and otherwise focus on fleshing out the missing parts of the spec.

Eventually, we'll want to also add a Reference section in here with no-bullshit explanations of all the components.

See some follow up notes in #1445

ebuchman force-pushed the bucky/docs-core branch from 64e3f9f to 1178589 Compare June 26, 2018 21:31

ebuchman and others added 17 commits June 27, 2018 07:15

example app1

9233e35

docs: update readme structure with example apps

7e50e7d

docs/examples: templates for more examples

234d749

Added handler to app1

05db92f

finish docs for app1

b3b075c

consolidate files into appX.md

7566ac2

minor fixes

bc35c72

fix comment

cc4f254

more minor fixes

69a4737

more structure for app2 and app3 md

dde8abf

some reordering in app1 and app3

72c1381

update readme

e6aec82

Finish app1-3, app4 done minus staking

d1c9b8b

Remove unnecessary fields from simple account

c0445e5

Initialize coin metadata in app4 and fmt

e11e469

Revert changes to app1 router

41c61d2

move AnteHandler to app2 and some cleanup in app3

354f976

ebuchman force-pushed the bucky/docs-core branch from eb5dba6 to 6a5a8b4 Compare June 27, 2018 11:49

ebuchman added 8 commits June 27, 2018 07:51

update/finish app1 go and md

fc81c14

minor cleanup

6a5a8b4

more app1 cleanup

0987787

minor updates in App2

d6a01ba

working on app3

bd581b2

app3 ante handler

b335d3b

some cleanup, remove old files

0c5e3fd

app3 coin keeper

6bbe295

ebuchman changed the title ~~WIP: docs via example apps~~ docs via example apps Jun 27, 2018

rigelrozanski added T:Docs Changes and features related to documentation. wip labels Jun 27, 2018

alexanderbez reviewed Jun 28, 2018

View reviewed changes

AdityaSripal and others added 4 commits June 28, 2018 15:08

Improved apps with better handling/routing and simpler MsgIssue

98be0e7

Added some documentation

e3f38b6

Appease linter

4e2eb24

Merge pull request #1434 from cosmos/bucky-aditya/docs-core

3a96f8f

Bucky aditya/docs core

ebuchman mentioned this pull request Jun 28, 2018

WIP: Fedekunze/update rtd #1054

Closed

4 tasks

ebuchman added 3 commits June 28, 2018 19:06

fixes from review

e8946e9

address TODOs in app 1 and 2

e708104

more app1/app2 cleanup

778b102

ebuchman mentioned this pull request Jun 29, 2018

docs: summary of some TODOs in the core #1445

Closed

ebuchman added 2 commits June 28, 2018 20:17

fill in app3 todos. simplify app3.go

d0efeb1

satisfy linter

d1a42e0

alexanderbez previously approved these changes Jun 29, 2018

View reviewed changes

ebuchman dismissed alexanderbez’s stale review via 1d4d9e9 June 29, 2018 03:32

ebuchman added 5 commits June 28, 2018 23:42

simplify and complete app3

1d4d9e9

template app4.md. simplify app4.go

f405bdf

started app5

12a1807

cleanup _attic

822ebdb

add links for modules and clients

4e87cdf

minor fix

a7cdea5

ebuchman merged commit a88b6b9 into develop Jun 29, 2018

ebuchman deleted the bucky/docs-core branch June 29, 2018 06:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs via example apps #1376

docs via example apps #1376

ebuchman commented Jun 26, 2018

codecov bot commented Jun 26, 2018 •

edited

Loading

ebuchman commented Jun 27, 2018

alexanderbez left a comment

alexanderbez Jun 27, 2018

ebuchman Jun 28, 2018

alexanderbez Jun 27, 2018

alexanderbez Jun 27, 2018

alexanderbez Jun 28, 2018

alexanderbez Jun 28, 2018

alexanderbez Jun 28, 2018

alexanderbez Jun 28, 2018

ebuchman Jun 28, 2018

alexanderbez Jun 28, 2018

ebuchman Jun 28, 2018

alexanderbez Jun 29, 2018

alexanderbez Jun 28, 2018

alexanderbez Jun 28, 2018

ebuchman Jun 28, 2018

alexanderbez left a comment

ebuchman commented Jun 29, 2018


		We have only a single message type, so just one message-specific function to define, `handleMsgSend`.

		Note this handler has unfettered access to the store specified by the capability key `keyAcc`. So it must also define items in the store are encoded.

docs via example apps #1376

docs via example apps #1376

Conversation

ebuchman commented Jun 26, 2018

codecov bot commented Jun 26, 2018 • edited Loading

Codecov Report

ebuchman commented Jun 27, 2018

alexanderbez 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

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

alexanderbez left a comment

Choose a reason for hiding this comment

ebuchman commented Jun 29, 2018

codecov bot commented Jun 26, 2018 •

edited

Loading