First thing I wanted to mention - we have some helpers that make sure a certain value is set, or they panic/return error. How do we deal with that in case of a struct that is supposed to have all these parameters? I guess we should revisit how this is being populated, so that we're guaranteed that parameters are always there when we need them, or we fail somewhere on framework-level.

I quite liked the signature with context and not context included in the environment, cause then the usage of context is not as transparent.

I would suggest to group all these properties in a struct and indeed use them where needed, maybe even by interface, but still keep them in a context. Or extend the signature to be
Check(ctx Context, info BlockInfo, store KVStore, tx Tx) (*CheckResult, error)

IMHO the context should be visible and not hidden in another struct.

Check(ctx Context, info BlockInfo, store KVStore, tx Tx) (*CheckResult, error)

A bit longer function signature, but very clear. I like

The old weave.Context contained very different things at once:

• BlockInfo
• AuthN
• Logger
• Extension point to enrich context

Moving some of this into structured data makes sense for me. Ideally, I would like to see AuthN as structured data (weave.Conditions), too.
The logger should not be passed via context but as a constructor argument to the handlers/decorators.

Check(ctx Context, info BlockInfo, store KVStore, tx Tx) (*CheckResult, error) makes BlockInfo more prominent than needed. I think passing it via a new weaveContext type would be good enough.

@alpe So you prefer some struct like

type Environment struct {
  Auth context.Context
  Block BlockInfo
}

over the second arg?

I like the idea of passing in the logger in the constructor, but we currently set info on it for every call:

	ctx := weave.WithLogInfo(b.BlockContext(),
		"call", "deliver_tx",
		"path", weave.GetPath(tx))

https://github.com/iov-one/weave/blob/master/app/base.go#L42-L45

I would actually like to add the height there, for nice structured logging. The only actual logging in current code is not in any Handlers, but in the generic LoggingDecorator:

	logger := weave.GetLogger(ctx).With("duration", delta/time.Microsecond)
	// ...
	logger.Info(msg)

https://github.com/iov-one/weave/blob/master/x/utils/logging.go

I could pass it in as a top-level argument to Deliver, but decided to hide it "somewhere". Better suggestions where to hide it are welcome.

Also, I do not think BlockInfo (maybe renamed) is too prominent. It is central to the control flow of many modules - aswap, escrow, gov - and controls whether or not transations succeed.

The other essential info is h.auth.HasAddress(ctx), which puts the onus on the x.Authenticator implementations to handle the context, rather than making the "smart contracts" understand what we throw in the context (as now).

I think anything that is critical to the control flow should be explicit. You can see how cosmos-sdk throws everything into their Context mod:

func NewContext(ms MultiStore, header abci.Header, isCheckTx bool, logger log.Logger) Context {
	c := Context{
		Context: context.Background(),
		pst:     newThePast(),
		gen:     0,
	}

	c = c.WithMultiStore(ms)
	c = c.WithBlockHeader(header)
	c = c.WithChainID(header.ChainID)
	c = c.WithIsCheckTx(isCheckTx)
	c = c.WithTxBytes(nil)
	c = c.WithLogger(logger)
	c = c.WithVoteInfos(nil)
	c = c.WithGasMeter(stypes.NewInfiniteGasMeter())
	c = c.WithMinGasPrices(DecCoins{})
	c = c.WithConsensusParams(nil)
	c = c.WithEventManager(NewEventManager())

	return c
}

https://github.com/cosmos/cosmos-sdk/blob/master/types/context.go#L38-L58

Taking our current approach to it's logical conclusion. With such items as the reference to the database, as well as the gasMeter to abort transactions early, being stored in some opaque Context object, which is not actually context.Context, but a cosmos-specific "improvement"

This should highlight some of the motivation of simplifying into a much more standard Context with no "magic info" assumed to be in there. Nice discussions on #835 about possible new uses with such a Context

Thanks for your additional infos and comments. It is very good that we address this issue before it becomes a problem. I had some structured data context like this in mind:

type WeaveContext struct{
	context.Context  // transient unstructured data
	AuthN []Condition
	Block BlockInfo
	Logger Logger // can not be removed easily
}

If we go this route, why not just Deliver(request Request) (*DeliverResult, error)?

And then a struct:

type Request struct {
  Auth []Address
  Block BlockInfo
  Ctx context.Context
  DB CacheableKVStore
  Logger Logger
  Tx Tx
}

I would rather collapse all args into one struct. Or all as separate args. I do see the value of this approach actually.