Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explore a new TxBuilder for SIGN_MODE_DIRECT_AUX (for tipper) #10443

Closed
amaury1093 opened this issue Oct 26, 2021 · 1 comment · Fixed by #10455 or #10547
Closed

Explore a new TxBuilder for SIGN_MODE_DIRECT_AUX (for tipper) #10443

amaury1093 opened this issue Oct 26, 2021 · 1 comment · Fixed by #10455 or #10547
Assignees

Comments

@amaury1093
Copy link
Contributor

amaury1093 commented Oct 26, 2021

Problem

The current TxBuilder has methods that have nothing to do for a tipper: SetFeePayer, SetFeeAmount, SetGas... It's confusing for a tipper TxBuilder to see these methods.

Moreover, it complicates usage of the same SignModeHandler in server and client side (#10208 (comment))

Solution

Try to think of a AuxTxBuilder that would be targetted at outputting something ready for the feepayer to consume. It can be:

  • a tx.Tx without Fee
  • or maybe a SignDocDirectAux + a signature
  • or something in between

Related to this:

  • we could refactor the existing client.TxBuilder to not support StdTxBuilder. client.TxBuilder would support DIRECT and LEGACY_AMINO_JSON, and AuxTxBuilder would support DIRECT_AUX
@amaury1093 amaury1093 self-assigned this Oct 27, 2021
@amaury1093 amaury1093 mentioned this issue Oct 28, 2021
19 tasks
@mergify mergify bot closed this as completed in #10455 Nov 11, 2021
mergify bot pushed a commit that referenced this issue Nov 11, 2021
<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

Closes: #10443 

For creating an intermediary/auxiliary tx (e.g. by the tipper in tipped transactions), using the existing `client.TxBuilder` is awkward. We propose a new client-side builder for this purpose.

API Usage (e.g. how the tipper would programmtically use this):
```go
// Note: there's no need to use clientCtx.TxConfig anymore!

bldr := clienttx.NewAuxTxBuilder()
err := bldr.SetMsgs(msgs...)
bldr.SetAddress("cosmos1...")
bldr.SetMemo(...)
bldr.SetTip(...)
bldr.SetPubKey(...)
err := bldr.SetSignMode(...) // DIRECT_AUX or AMINO, or else error
// ... other setters are available

// Get the bytes to sign.
signBz, err := bldr.GetSignBytes()

// Sign the bz using your favorite method.
sig, err := privKey.sign(signBz)

// Set the signature
bldr.SetSig(sig)

// Get the final auxSignerData to be sent to the fee payer
auxSignerData, err:= bldr.GetAuxSignerData()
```

auxSignerData is a protobuf message, whose JSON reprensentation looks like:

```json
{
  "address": "cosmos1...",
  "mode": "SIGN_MODE_{DIRECT_AUX,LEGACY_AMINO_JSON}",
  "sign_doc": {
    "body_bytes": "{base64 bytes}",
    "public_key": {
      "@type": "cosmos.sepc256k1.PubKey",
      "key": "{base64 bytes}"
    },
    "chain_id": "...",
    "account_number": 24,
    "sequence": 42,
    "tip": {
      "amount": [{ "denom": "uregen", "amount": 1000 }],
      "tipper": "cosmos1..."
    }
  },
  "sig": "{base64 bytes}"
}
```

Then the fee payer would use the TxBuilder to construct the final TX, with a new helper method: `AddAuxSignerData`:

```go
// get auxSignerData from AuxTxBuilder
auxSignerData := ...

txBuilder := txConfig.NewTxBuilder()
err := txBuilder.AddAuxSignerData(auxSignerData)

// Set fee payer data
txBuilder.SetFee()
txBuilder.SetGasLimit()
txBuilder.SetFeePayer()

sigs, err := txBuilder.GetSignaturesV2()
auxSig := sigs[0] // the aux signer's signature

// Set all signer infos (1st round of calling SetSignatures)
txBuilder.SetSignatures(
  auxSig,                   // The aux SignatureV2
  signing.SignatureV2{...}, // The feePayer's SignatureV2
)

// Sign
signBz, err = encCfg.TxConfig.SignModeHandler().GetSignBytes(...)
feepayerSig, err := feepayerPriv.Sign(signBz)

// Set all signatures (2nd round of calling SetSignatures)
txBuilder.SetSignatures(
  auxSig,                   // The aux SignatureV2
  signing.SignatureV2{feepayerSig, ...}, // The feePayer's SignatureV2
)
```

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
@amaury1093 amaury1093 reopened this Nov 15, 2021
@amaury1093
Copy link
Contributor Author

Re-opened, as the previous PR wasn't finished. Pending on #10547

blewater pushed a commit to e-money/cosmos-sdk that referenced this issue Dec 8, 2021
<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

Closes: cosmos#10443 

For creating an intermediary/auxiliary tx (e.g. by the tipper in tipped transactions), using the existing `client.TxBuilder` is awkward. We propose a new client-side builder for this purpose.

API Usage (e.g. how the tipper would programmtically use this):
```go
// Note: there's no need to use clientCtx.TxConfig anymore!

bldr := clienttx.NewAuxTxBuilder()
err := bldr.SetMsgs(msgs...)
bldr.SetAddress("cosmos1...")
bldr.SetMemo(...)
bldr.SetTip(...)
bldr.SetPubKey(...)
err := bldr.SetSignMode(...) // DIRECT_AUX or AMINO, or else error
// ... other setters are available

// Get the bytes to sign.
signBz, err := bldr.GetSignBytes()

// Sign the bz using your favorite method.
sig, err := privKey.sign(signBz)

// Set the signature
bldr.SetSig(sig)

// Get the final auxSignerData to be sent to the fee payer
auxSignerData, err:= bldr.GetAuxSignerData()
```

auxSignerData is a protobuf message, whose JSON reprensentation looks like:

```json
{
  "address": "cosmos1...",
  "mode": "SIGN_MODE_{DIRECT_AUX,LEGACY_AMINO_JSON}",
  "sign_doc": {
    "body_bytes": "{base64 bytes}",
    "public_key": {
      "@type": "cosmos.sepc256k1.PubKey",
      "key": "{base64 bytes}"
    },
    "chain_id": "...",
    "account_number": 24,
    "sequence": 42,
    "tip": {
      "amount": [{ "denom": "uregen", "amount": 1000 }],
      "tipper": "cosmos1..."
    }
  },
  "sig": "{base64 bytes}"
}
```

Then the fee payer would use the TxBuilder to construct the final TX, with a new helper method: `AddAuxSignerData`:

```go
// get auxSignerData from AuxTxBuilder
auxSignerData := ...

txBuilder := txConfig.NewTxBuilder()
err := txBuilder.AddAuxSignerData(auxSignerData)

// Set fee payer data
txBuilder.SetFee()
txBuilder.SetGasLimit()
txBuilder.SetFeePayer()

sigs, err := txBuilder.GetSignaturesV2()
auxSig := sigs[0] // the aux signer's signature

// Set all signer infos (1st round of calling SetSignatures)
txBuilder.SetSignatures(
  auxSig,                   // The aux SignatureV2
  signing.SignatureV2{...}, // The feePayer's SignatureV2
)

// Sign
signBz, err = encCfg.TxConfig.SignModeHandler().GetSignBytes(...)
feepayerSig, err := feepayerPriv.Sign(signBz)

// Set all signatures (2nd round of calling SetSignatures)
txBuilder.SetSignatures(
  auxSig,                   // The aux SignatureV2
  signing.SignatureV2{feepayerSig, ...}, // The feePayer's SignatureV2
)
```

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant