Skip to content

Commit

Permalink
fix(textual): Use HEX instead of base64 (cosmos#12954)
Browse files Browse the repository at this point in the history
## Description

ref: cosmos#12910 (comment)



---

### 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/main/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/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/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)
  • Loading branch information
amaury1093 authored Aug 22, 2022
1 parent 9412bcd commit 2496433
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 16 deletions.
2 changes: 1 addition & 1 deletion docs/architecture/adr-050-sign-mode-textual-annex1.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ Rendered as either ISO8601 (`2021-01-01T12:00:00Z`).

### bytes

- Bytes are rendered in base64.
- Bytes are rendered in hexadecimal, all capital letters, without the `0x` prefix.

### address bytes

Expand Down
8 changes: 4 additions & 4 deletions docs/architecture/adr-050-sign-mode-textual.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ We define "transaction envelope" as all data in a transaction that is not in the
```
Chain ID: <string>
Account number: <uint64>
*Public Key: <base64_string>
*Public Key: <hex_string>
Sequence: <uint64>
<TxBody> // See #8.
Fee: <coins> // See value renderers for coins encoding.
Expand All @@ -112,7 +112,7 @@ Tip: <string>
*<repeated Any>
*This transaction has <int> other signers: // Skipped if there is only one signer
*Signer (<int>/<int>):
*Public Key: <base64_string>
*Public Key: <hex_string>
*Sequence: <uint64>
*End of other signers
```
Expand Down Expand Up @@ -222,7 +222,7 @@ SIGN_MODE_TEXTUAL:
```
Chain ID: simapp-1
Account number: 10
*Public Key: iQ...== // Base64 pubkey
*Public Key: iQ...== // Hex pubkey
Sequence: 2
This transaction has 1 message:
Message (1/1): bank v1beta1 send coins
Expand Down Expand Up @@ -481,7 +481,7 @@ SIGN_MODE_TEXTUAL is purely additive, and doesn't break any backwards compatibil

### Negative

- Some fields are still encoded in non-human-readable ways, such as public keys in base64.
- Some fields are still encoded in non-human-readable ways, such as public keys in hexadecimal.
- New ledger app needs to be released, still unclear

### Neutral
Expand Down
7 changes: 4 additions & 3 deletions tx/textual/valuerenderer/bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package valuerenderer

import (
"context"
"encoding/base64"
"encoding/hex"
"io"
"strings"

"google.golang.org/protobuf/reflect/protoreflect"
)
Expand All @@ -14,7 +15,7 @@ type bytesValueRenderer struct{}
var _ ValueRenderer = bytesValueRenderer{}

func (vr bytesValueRenderer) Format(ctx context.Context, v protoreflect.Value, w io.Writer) error {
_, err := io.WriteString(w, base64.StdEncoding.EncodeToString(v.Bytes()))
_, err := io.WriteString(w, strings.ToUpper(hex.EncodeToString(v.Bytes())))
return err
}

Expand All @@ -24,7 +25,7 @@ func (vr bytesValueRenderer) Parse(_ context.Context, r io.Reader) (protoreflect
return protoreflect.ValueOfBytes([]byte{}), err
}

data, err := base64.StdEncoding.DecodeString(string(formatted))
data, err := hex.DecodeString(string(formatted))
if err != nil {
return protoreflect.ValueOfBytes([]byte{}), err
}
Expand Down
24 changes: 16 additions & 8 deletions tx/textual/valuerenderer/bytes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package valuerenderer_test

import (
"context"
"encoding/hex"
"encoding/base64"
"encoding/json"
"os"
"strings"
Expand All @@ -12,34 +12,42 @@ import (
"google.golang.org/protobuf/reflect/protoreflect"
)

func TestFormatBytes(t *testing.T) {
func TestBytesJsonTestCases(t *testing.T) {
var testcases []bytesTest
// Bytes.json contains bytes that are represented in base64 format, and
// their expected results in hex.
raw, err := os.ReadFile("../internal/testdata/bytes.json")
require.NoError(t, err)

err = json.Unmarshal(raw, &testcases)
require.NoError(t, err)

for _, tc := range testcases {
data, err := hex.DecodeString(tc.hex)
data, err := base64.StdEncoding.DecodeString(tc.base64)
require.NoError(t, err)

r, err := valueRendererOf(data)
valrend, err := valueRendererOf(data)
require.NoError(t, err)

b := new(strings.Builder)
err = r.Format(context.Background(), protoreflect.ValueOfBytes(data), b)
err = valrend.Format(context.Background(), protoreflect.ValueOfBytes(data), b)
require.NoError(t, err)
require.Equal(t, tc.expRes, b.String())
require.Equal(t, tc.hex, b.String())

// Round trip
r := strings.NewReader(tc.hex)
val, err := valrend.Parse(context.Background(), r)
require.NoError(t, err)
require.Equal(t, tc.base64, base64.StdEncoding.EncodeToString(val.Bytes()))
}
}

type bytesTest struct {
hex string
expRes string
base64 string
}

func (t *bytesTest) UnmarshalJSON(b []byte) error {
a := []interface{}{&t.hex, &t.expRes}
a := []interface{}{&t.hex, &t.base64}
return json.Unmarshal(b, &a)
}

0 comments on commit 2496433

Please sign in to comment.