basecli AppTx - Signer should be added when reading flags

[rigelrozanski]

Plugins sometimes require the signer address to function properly. This address should be returned in the TxInput of AppTx when returned from AppTx Helper command ReadAppTxFlags(). Right now signers are added later before broadcast in the BroadcastAppTx as a part of the AddSigner function. This can simply removed after its been added to ReadAppTxFlags()

https://github.com/tendermint/basecoin/blob/develop/cmd/basecli/commands/cmds.go#L148

[ethanfrey]

Please show me the code in trackomatron where this is causing a problem.

The signer comes from checking the key manager, tried to handle that all in one step, but I guess I could split it up in two parts if it's better. Just want to see a concrete problem with the current structure.

[rigelrozanski]

https://github.com/tendermint/trackomatron/blob/f257a5d3f146c5c8d6097a129f19bc5a402a6a9d/cmd/trackocli/tx/profile.go#L67-L102

        gas, fee, txInput, err := bcmd.ReadAppTxFlags()
	if err != nil {
		return err
	}

	// Retrieve the app-specific flags/args
	var name string
	if TBTx == invoicer.TBTxProfileOpen {
		if len(args) != 1 {
			return trcmn.ErrCmdReqArg("name")
		}
		name = args[0]
	}

	//TODO get the address here from txInput once changed in basecoin
	data := profileTx(TBTx, txcmd.GetSigner().Address(), name)

	// Create AppTx and broadcast
	tx := &btypes.AppTx{
		Gas:   gas,
		Fee:   fee,
		Name:  invoicer.Name,
		Input: txInput,
		Data:  data,
	}
	res, err := bcmd.BroadcastAppTx(tx)
	if err != nil {
		return err
	}

Within any trackomatron profile is linked to a basecoin pubkey address, aka this information needs to be sent in with the transaction in order to be interpreted used on the server-side processing. Right now, as shown in the above code, we generate the txInput before we generate the custom plugin tx bytes (which need the address). Within the txInput there is a field for the address and the pubkey however, they are left empty at generation and only filled in later in the AddSigner function which is a part of BroadcastAppTx which is called here... but this is after generating the txInput... I have a workaround right now which is to just the signer directly as I've done here, it's just unnecessary we should only need to call the function once.. not to mention that it's confusing that those fields exist but they're just not filled in... Makes sense?

[ethanfrey]

Yeah, I figured all this info could be filled in at the last step.

Now I realize, we need to actually build some opaque []byte for the app, which will need this info. So this doesn't work.

Sure, pull it out.

My thought was a bit that light-client needs to support unsigned tx to work with dummy / merkleeyes / other example apps. So I didn't make the signing name fully required from the get-go. But clearly in basecoin, it is always required. I just didn't change my thinking.

Yes, please do the Signer / sig stuff early on here, as it is always needed.

[ethanfrey]

Closed with #140