feat(textual): add renderers for message and string

[kirbyquerby]

Description

Updates #12713

This change adds value renderers for message and string types. I also added some message types to testpb with nested messages + snake-cased names and without repeated fields (which aren't implemented yet).

---

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...

• [x] included the correct type prefix in the PR title
• [x] added ! to the type prefix if API or client breaking change
• [x] targeted the correct branch (see PR Targeting)
• [x] provided a link to the relevant issue or specification
• [x] followed the guidelines for building modules
• [x] included the necessary unit and integration tests
• [ ] added a changelog entry to CHANGELOG.md
• [x] included comments for documenting Go code
• [x] updated the relevant documentation or specification
• [x] 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 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)

[codecov[bot]]

Codecov Report

Merging #12878 (a7cf4d5) into main (695d7f5) will increase coverage by 0.22%. The diff coverage is 61.66%.

:exclamation: Current head a7cf4d5 differs from pull request most recent head f47773d. Consider uploading reports for the commit f47773d to get more accurate results

@@            Coverage Diff             @@
##             main   #12878      +/-   ##
==========================================
+ Coverage   55.68%   55.91%   +0.22%     
==========================================
  Files         644      652       +8     
  Lines       54844    55071     +227     
==========================================
+ Hits        30542    30791     +249     
+ Misses      21839    21808      -31     
- Partials     2463     2472       +9     

Impacted Files	Coverage Δ
tx/textual/valuerenderer/valuerenderer.go	65.21% <36.36%> (ø)
tx/textual/valuerenderer/string.go	60.00% <60.00%> (ø)
tx/textual/valuerenderer/message.go	68.18% <68.18%> (ø)
x/distribution/keeper/fee_pool.go	0.00% <0.00%> (-66.67%)	:arrow_down:
x/slashing/keeper/msg_server.go	35.48% <0.00%> (-25.81%)	:arrow_down:
client/grpc_query.go	0.00% <0.00%> (-15.39%)	:arrow_down:
x/distribution/keeper/msg_server.go	15.44% <0.00%> (-11.71%)	:arrow_down:
x/distribution/keeper/allocation.go	86.66% <0.00%> (-10.44%)	:arrow_down:
snapshots/manager.go	47.58% <0.00%> (-9.33%)	:arrow_down:
x/slashing/keeper/keeper.go	45.09% <0.00%> (-7.85%)	:arrow_down:
... and 49 more

[kirbyquerby]

The lint errors are in unchanged files.

[peterbourgon]

The ADR says that

all strings MUST only contain ASCII characters in the 32-127 range.

Is this requirement satisfied by the code in this PR?

[kirbyquerby]

The ADR says that

all strings MUST only contain ASCII characters in the 32-127 range.

Is this requirement satisfied by the code in this PR?

@peterbourgon As the current PR is, not necessarily. There are a few places in this PR where invalid characters might be written:

• The message renderer itself won't add invalid characters, but it's possible that it will write invalid characters if one of the ValueRenderers it calls to render the message fields returns them.
  • It seems reasonable to assume that other renderers will behave well.
• The message renderer will write newline characters, as specified in the annex.
  • Not sure whether this is an issue.
• The string renderer will currently write invalid characters if the rendered string contains them. This follows the instructions in the annex, which state:

  Strings are rendered as-is.

  • There aren't any instructions for what to do if a string contains invalid characters. @amaurym should there be some special handling for this case?

[peterbourgon]

@kirbyquerby

all strings MUST only contain ASCII characters in the 32-127 range

Strings are rendered as-is.

It's a mutual exclusion cage match!! Two requirements enter, only one requirement can leave!! 😉

Go strings are allowed to contain basically any byte. If the output of this renderer should only contain bytes between 0x20 and 0x7F, then you need a func Sanitize(string) string that ensures that invariant. Here's a naïve example:

func sanitize(s string) string {
	var (
		asBytes = []byte(s)
		minByte = byte(0x20)
		maxByte = byte(0x7F)
		replace = byte('X')
	)
	for i, b := range asBytes {
		if b < minByte || b > maxByte {
			asBytes[i] = replace
		}
	}
	return string(asBytes)
}

(There are probably bugs.)

[JimLarson]

Regarding non-ASCII or ASCII control characters in strings: the more important of the conflicting directives in the spec is the restriction that the rendered text be simple non-control, non-newline ASCII, because this is the restriction of the target device. Although Go strings may be arbitrary sequences of bytes, protocol buffer string fields may only be utf8-encoded sequences of Unicode code points (ref). There is no reason to sacrifice bijectivity here, as there are many unicode-to-ascii quotation schemes with guarantee lossless round-trips.

The Go standard library provides strconv.QuoteToASCII() and strconv.Unquote() which does exactly this. However, it quotes to the Go string literal format, which puts JS at a disadvantage, and the exact specification is hard to summarize - so much so that I'm not entirely sure that it quotes all control characters. Instead, I'd recommend using the simpler JSON string literal format such that encoding/json.Unmarshal() can be the unquote/parse function, and the quote/format function can be relatively simple custom code. This is relatively easy to implement and test, and simple to specify.

[JimLarson]

ASCII characters 32-127 are precisely the non-control characters (and newline is a control character). The package documentation suggests that QuoteToASCII produces only the ASCII printable (i.e. non-control) characters, but defers to unicode.IsPrint() for specifics, and I don't feel like looking up the definition of Unicode character classes to parse that definition.

[amaury1093]

Putting in draft for now, there are some spec changes that might happen. Though @kirbyquerby you can continue work on the internal struct (that we discueed Monday) if you wish

[amaury1093]

@kirbyquerby Is it okay if @JimLarson takes over this PR? He's working on updating the SPEC in #13434, and it makes sense for him to work on this implementation PR hand-in-hand.

[kirbyquerby]

@amaurym @JimLarson Sure, that works for me!

[amaury1093]

closing in favor of #13510