feat(client/v2/offchain): sign and verify file #18626
Conversation
WalkthroughThe update implements a new feature for off-chain signing and verification of arbitrary data, specifically targeting files. This enhancement involves several parts of the client, adding commands and utilities to sign and verify files using different encoding and marshalling methods. It allows for the handling of off-chain messages, transaction building, and signature verification, broadening the scope of off-chain capabilities with respect to file management within a Cosmos SDK application context. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
I think the codec and crypto packages are needed as dependency. Same with the client package as this is heavy dependant on the client context. |
| // AppDomain is the application requesting off-chain message signing | ||
| string appDomain = 1; | ||
| // Signer is the sdk.AccAddress of the message signer | ||
| string signerAddress = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; |
There was a problem hiding this comment.
| string signerAddress = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; | |
| string signer = 2 [(cosmos_proto.scalar) = "cosmos.AddressString"]; |
nit
There was a problem hiding this comment.
maybe is a good idea to modify it in the CIP as well.
this is part of client v2 so maybe its removed later on. i think that is fine since its bleeding elsewhere as well |
Co-authored-by: Marko <marbar3778@yahoo.com>
Ok. I'm only concerned then with |
Aren't we able to use x/tx instead for some data? With #18580, we should be able to use: for signing and have the sdk keyring implement that. |
|
should we implement this for multisig accounts? I don' think it will be really used, plus this builder it's meant to be deleted once the v2 builder is developed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
client/v2/offchain/verify_test.go
Outdated
| func Test_unmarshal(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| digest []byte | ||
| fileFormat string | ||
| }{ | ||
| { | ||
| name: "check", | ||
| digest: []byte(`{"body":{"messages":[{"@type":"/offchain.MsgSignArbitraryData","appDomain":"simd","signer":"cosmos1rt2xyymh5pvycl8dc00et4mxgr4cpzcdlk8ped","data":"{\n\t\"name\": \"John\",\n\t\"surname\": \"Connor\",\n\t\"age\": 15\n}\n"}],"memo":"","timeoutHeight":"0","extensionOptions":[],"nonCriticalExtensionOptions":[]},"authInfo":{"signerInfos":[{"publicKey":{"@type":"/cosmos.crypto.secp256k1.PubKey","key":"A06XPD6ML7BHSWHsc2u5EtkCXzsCNmhgPaDdNCp5nPF2"},"modeInfo":{"single":{"mode":"SIGN_MODE_DIRECT"}},"sequence":"0"}],"fee":{"amount":[],"gasLimit":"0","payer":"","granter":""},"tip":null},"signatures":["hx8Qo6xZ/Ie0d1TFtiVxSK1rUsRKDEiv1IdcgbkSGYgePYZl6aHJxpSxQDXdIeoZiPeIdrsTkkgjmH4wv2BBdw=="]}`), | ||
| fileFormat: "json", | ||
| }, | ||
| { | ||
| name: "signMode direct text", | ||
| digest: []byte("body:{messages:{[/offchain.MsgSignArbitraryData]:{appDomain:\"simd\" signer:\"cosmos15r8vphexk8tnu6gvq0a5dhfs3j06ht9kux78rp\" data:\"{\\n\\t\\\"name\\\": \\\"John\\\",\\n\\t\\\"surname\\\": \\\"Connor\\\",\\n\\t\\\"age\\\": 15\\n}\\n\"}}} auth_info:{signer_infos:{public_key:{[/cosmos.crypto.secp256k1.PubKey]:{key:\"\\x03\\xa85\\xb37ړO\\xd6x\\xb5\\x9f\\xb1\\x83\\x92`\\x1b\\xf7Q\\xd0<v\\xdf\\xc4}\\x82\\xcb\\x1eG\\xf5c\\x9c\\xad\"}} mode_info:{single:{mode:SIGN_MODE_DIRECT}}} fee:{}} signatures:\"\\x05\\xacz\\xfa1\\xba\\xa4d\\xc8\\xfa\\xdcT\\xa8B7\\xa4\\xc7餣jf\\xee\\x1e\\xecp\\x07\\xfe\\xb61\\\"Fd\\x19z\\x89(8&\\xf0J\\xe2\\xdd\\\"C\\xe8\\x7ffH5\\r\\xd8E\\xb5TH\\x80v\\x9dNew:\\x03\""), | ||
| fileFormat: "text", | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| got, err := unmarshal(tt.digest, tt.fileFormat) | ||
| require.NoError(t, err) | ||
| require.NotNil(t, got) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider adding negative test cases to Test_unmarshal to ensure that the unmarshalling function properly handles invalid data or formats.
client/v2/offchain/cli.go
Outdated
| // VerifyFile verifies given file with given key. | ||
| func VerifyFile() *cobra.Command { | ||
| cmd := &cobra.Command{ | ||
| Use: "verify-file <keyName> <fileName>", | ||
| Short: "Verify a file.", | ||
| Long: "Verify a previously signed file with the given key.", | ||
| Args: cobra.ExactArgs(2), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| clientCtx, err := client.GetClientQueryContext(cmd) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| bz, err := os.ReadFile(args[1]) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fileFormat, _ := cmd.Flags().GetString(flagFileFormat) | ||
|
|
||
| err = Verify(clientCtx, bz, fileFormat) | ||
| if err == nil { | ||
| cmd.Println("Verification OK!") | ||
| } | ||
| return err | ||
| }, | ||
| } | ||
|
|
||
| cmd.PersistentFlags().String(flagFileFormat, "json", "Choose whats the file format to be verified (json|text)") | ||
| return cmd |
There was a problem hiding this comment.
The VerifyFile command also follows the standard pattern for Cobra commands. It reads the file and calls the Verify function with the appropriate parameters. Points to consider:
- The
RunEfunction assumes that any error returned from theVerifyfunction is a verification failure. However, it could also be an error related to reading the file or other issues. It might be beneficial to differentiate between verification failures and other types of errors to provide clearer feedback to the user. - The
flagFileFormatis retrieved, but similar to theSignFilecommand, there is no validation to ensure that the provided value is within the expected range or format. This could lead to unexpected behavior if an invalid value is passed.
Adding error handling to differentiate between verification failures and other errors, as well as validation for the flagFileFormat, would improve the robustness of the command.
client/v2/offchain/sign_test.go
Outdated
| func Test_sign(t *testing.T) { | ||
| k := keyring.NewInMemory(getCodec()) | ||
|
|
||
| ctx := client.Context{ | ||
| Keyring: k, | ||
| TxConfig: MakeTestTxConfig(), | ||
| AddressCodec: address.NewBech32Codec("cosmos"), | ||
| } | ||
|
|
||
| type args struct { | ||
| ctx client.Context | ||
| fromName string | ||
| digest string | ||
| signMode apitxsigning.SignMode | ||
| } | ||
| tests := []struct { | ||
| name string | ||
| args args | ||
| wantErr bool | ||
| }{ | ||
| { | ||
| name: "signMode direct", | ||
| args: args{ | ||
| ctx: ctx, | ||
| fromName: "direct", | ||
| digest: "Hello world!", | ||
| signMode: apitxsigning.SignMode_SIGN_MODE_DIRECT, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "signMode textual", | ||
| args: args{ | ||
| ctx: ctx, | ||
| fromName: "textual", | ||
| digest: "Hello world!", | ||
| signMode: apitxsigning.SignMode_SIGN_MODE_TEXTUAL, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "signMode legacyAmino", | ||
| args: args{ | ||
| ctx: ctx, | ||
| fromName: "legacy", | ||
| digest: "Hello world!", | ||
| signMode: apitxsigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "signMode direct aux", | ||
| args: args{ | ||
| ctx: ctx, | ||
| fromName: "direct-aux", | ||
| digest: "Hello world!", | ||
| signMode: apitxsigning.SignMode_SIGN_MODE_DIRECT_AUX, | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| _, err := k.NewAccount(tt.args.fromName, mnemonic, tt.name, "m/44'/118'/0'/0/0", hd.Secp256k1) | ||
| require.NoError(t, err) | ||
|
|
||
| got, err := sign(tt.args.ctx, tt.args.fromName, tt.args.digest, tt.args.signMode) | ||
| if !tt.wantErr { | ||
| require.NoError(t, err) | ||
| require.NotNil(t, got) | ||
| } else { | ||
| require.Error(t, err) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
The test cases in Test_sign cover various sign modes and expected outcomes. It would be beneficial to include comments explaining why an error is expected for certain test cases, such as signMode direct aux.
client/v2/offchain/sign.go
Outdated
| // sign signs a digest with provided key and SignMode. | ||
| func sign(ctx client.Context, fromName, digest string, signMode apisigning.SignMode) (*apitx.Tx, error) { | ||
| keybase, err := keyring.NewAutoCLIKeyring(ctx.Keyring) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| pubKey, err := keybase.GetPubKey(fromName) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| addr, err := ctx.AddressCodec.BytesToString(pubKey.Address()) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| msg := &offchain.MsgSignArbitraryData{ | ||
| AppDomain: version.AppName, | ||
| Signer: addr, | ||
| Data: digest, | ||
| } | ||
|
|
||
| txBuilder := newBuilder(ctx.Codec) | ||
| err = txBuilder.setMsgs(msg) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| signerData := signerData{ | ||
| Address: addr, | ||
| ChainID: ExpectedChainID, | ||
| AccountNumber: ExpectedAccountNumber, | ||
| Sequence: ExpectedSequence, | ||
| PubKey: pubKey, | ||
| } | ||
|
|
||
| sigData := &SingleSignatureData{ | ||
| SignMode: signMode, | ||
| Signature: nil, | ||
| } | ||
|
|
||
| sig := OffchainSignature{ | ||
| PubKey: pubKey, | ||
| Data: sigData, | ||
| Sequence: ExpectedSequence, | ||
| } | ||
|
|
||
| sigs := []OffchainSignature{sig} | ||
| err = txBuilder.SetSignatures(sigs...) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| bytesToSign, err := getSignBytes( | ||
| context.Background(), ctx.TxConfig.SignModeHandler(), | ||
| signMode, signerData, txBuilder) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| signedBytes, err := keybase.Sign(fromName, bytesToSign, signMode) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| sigData.Signature = signedBytes | ||
|
|
||
| err = txBuilder.SetSignatures(sig) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| return txBuilder.GetTx(), nil |
There was a problem hiding this comment.
The sign function appears to correctly implement the signing process. However, consider adding error handling for the case where pubKey.Address() returns an address that cannot be converted to a string by ctx.AddressCodec.BytesToString. This could prevent a panic in case of an invalid address.
addr, err := ctx.AddressCodec.BytesToString(pubKey.Address())
if err != nil {
+ // Handle invalid address conversion
+ return nil, fmt.Errorf("failed to convert address to string: %w", err)
}Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| // sign signs a digest with provided key and SignMode. | |
| func sign(ctx client.Context, fromName, digest string, signMode apisigning.SignMode) (*apitx.Tx, error) { | |
| keybase, err := keyring.NewAutoCLIKeyring(ctx.Keyring) | |
| if err != nil { | |
| return nil, err | |
| } | |
| pubKey, err := keybase.GetPubKey(fromName) | |
| if err != nil { | |
| return nil, err | |
| } | |
| addr, err := ctx.AddressCodec.BytesToString(pubKey.Address()) | |
| if err != nil { | |
| return nil, err | |
| } | |
| msg := &offchain.MsgSignArbitraryData{ | |
| AppDomain: version.AppName, | |
| Signer: addr, | |
| Data: digest, | |
| } | |
| txBuilder := newBuilder(ctx.Codec) | |
| err = txBuilder.setMsgs(msg) | |
| if err != nil { | |
| return nil, err | |
| } | |
| signerData := signerData{ | |
| Address: addr, | |
| ChainID: ExpectedChainID, | |
| AccountNumber: ExpectedAccountNumber, | |
| Sequence: ExpectedSequence, | |
| PubKey: pubKey, | |
| } | |
| sigData := &SingleSignatureData{ | |
| SignMode: signMode, | |
| Signature: nil, | |
| } | |
| sig := OffchainSignature{ | |
| PubKey: pubKey, | |
| Data: sigData, | |
| Sequence: ExpectedSequence, | |
| } | |
| sigs := []OffchainSignature{sig} | |
| err = txBuilder.SetSignatures(sigs...) | |
| if err != nil { | |
| return nil, err | |
| } | |
| bytesToSign, err := getSignBytes( | |
| context.Background(), ctx.TxConfig.SignModeHandler(), | |
| signMode, signerData, txBuilder) | |
| if err != nil { | |
| return nil, err | |
| } | |
| signedBytes, err := keybase.Sign(fromName, bytesToSign, signMode) | |
| if err != nil { | |
| return nil, err | |
| } | |
| sigData.Signature = signedBytes | |
| err = txBuilder.SetSignatures(sig) | |
| if err != nil { | |
| return nil, err | |
| } | |
| return txBuilder.GetTx(), nil | |
| // sign signs a digest with provided key and SignMode. | |
| func sign(ctx client.Context, fromName, digest string, signMode apisigning.SignMode) (*apitx.Tx, error) { | |
| keybase, err := keyring.NewAutoCLIKeyring(ctx.Keyring) | |
| if err != nil { | |
| return nil, err | |
| } | |
| pubKey, err := keybase.GetPubKey(fromName) | |
| if err != nil { | |
| return nil, err | |
| } | |
| addr, err := ctx.AddressCodec.BytesToString(pubKey.Address()) | |
| if err != nil { | |
| // Handle invalid address conversion | |
| return nil, fmt.Errorf("failed to convert address to string: %w", err) | |
| } | |
| msg := &offchain.MsgSignArbitraryData{ | |
| AppDomain: version.AppName, | |
| Signer: addr, | |
| Data: digest, | |
| } | |
| txBuilder := newBuilder(ctx.Codec) | |
| err = txBuilder.setMsgs(msg) | |
| if err != nil { | |
| return nil, err | |
| } | |
| signerData := signerData{ | |
| Address: addr, | |
| ChainID: ExpectedChainID, | |
| AccountNumber: ExpectedAccountNumber, | |
| Sequence: ExpectedSequence, | |
| PubKey: pubKey, | |
| } | |
| sigData := &SingleSignatureData{ | |
| SignMode: signMode, | |
| Signature: nil, | |
| } | |
| sig := OffchainSignature{ | |
| PubKey: pubKey, | |
| Data: sigData, | |
| Sequence: ExpectedSequence, | |
| } | |
| sigs := []OffchainSignature{sig} | |
| err = txBuilder.SetSignatures(sigs...) | |
| if err != nil { | |
| return nil, err | |
| } | |
| bytesToSign, err := getSignBytes( | |
| context.Background(), ctx.TxConfig.SignModeHandler(), | |
| signMode, signerData, txBuilder) | |
| if err != nil { | |
| return nil, err | |
| } | |
| signedBytes, err := keybase.Sign(fromName, bytesToSign, signMode) | |
| if err != nil { | |
| return nil, err | |
| } | |
| sigData.Signature = signedBytes | |
| err = txBuilder.SetSignatures(sig) | |
| if err != nil { | |
| return nil, err | |
| } | |
| return txBuilder.GetTx(), nil | |
| } |
Perfect ty, no I just meant removing auth. |
If I remove the textual tests, it's easy as I can use |
|
@julienrbrt I've been able to remove |
client/v2/offchain/sign_test.go
Outdated
| func Test_sign(t *testing.T) { | ||
| k := keyring.NewInMemory(getCodec()) | ||
|
|
||
| ctx := client.Context{ | ||
| Keyring: k, | ||
| TxConfig: newTestConfig(t), | ||
| AddressCodec: address.NewBech32Codec("cosmos"), | ||
| } | ||
|
|
||
| type args struct { | ||
| ctx client.Context | ||
| fromName string | ||
| digest string | ||
| signMode apitxsigning.SignMode | ||
| } | ||
| tests := []struct { | ||
| name string | ||
| args args | ||
| wantErr bool | ||
| }{ | ||
| { | ||
| name: "signMode direct", | ||
| args: args{ | ||
| ctx: ctx, | ||
| fromName: "direct", | ||
| digest: "Hello world!", | ||
| signMode: apitxsigning.SignMode_SIGN_MODE_DIRECT, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "signMode textual", | ||
| args: args{ | ||
| ctx: ctx, | ||
| fromName: "textual", | ||
| digest: "Hello world!", | ||
| signMode: apitxsigning.SignMode_SIGN_MODE_TEXTUAL, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "signMode legacyAmino", | ||
| args: args{ | ||
| ctx: ctx, | ||
| fromName: "legacy", | ||
| digest: "Hello world!", | ||
| signMode: apitxsigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, | ||
| }, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "signMode direct aux", | ||
| args: args{ | ||
| ctx: ctx, | ||
| fromName: "direct-aux", | ||
| digest: "Hello world!", | ||
| signMode: apitxsigning.SignMode_SIGN_MODE_DIRECT_AUX, | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| } | ||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| _, err := k.NewAccount(tt.args.fromName, mnemonic, tt.name, "m/44'/118'/0'/0/0", hd.Secp256k1) | ||
| require.NoError(t, err) | ||
|
|
||
| got, err := sign(tt.args.ctx, tt.args.fromName, tt.args.digest, tt.args.signMode) | ||
| if !tt.wantErr { | ||
| require.NoError(t, err) | ||
| require.NotNil(t, got) | ||
| } else { | ||
| require.Error(t, err) | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
The Test_sign function correctly sets up an in-memory keyring and a client context for testing the sign function. The test cases cover different sign modes, and the assertions check for the presence or absence of errors as expected. However, the test case named "signMode direct aux" expects an error (wantErr: true), but there is no explanation as to why this error is expected. It would be beneficial to include a comment explaining the reason for the expected error to improve the maintainability of the test code.
{
name: "signMode direct aux",
args: args{
ctx: ctx,
fromName: "direct-aux",
digest: "Hello world!",
signMode: apitxsigning.SignMode_SIGN_MODE_DIRECT_AUX,
},
wantErr: true, // Please add a comment explaining why an error is expected here.
},Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| func Test_sign(t *testing.T) { | |
| k := keyring.NewInMemory(getCodec()) | |
| ctx := client.Context{ | |
| Keyring: k, | |
| TxConfig: newTestConfig(t), | |
| AddressCodec: address.NewBech32Codec("cosmos"), | |
| } | |
| type args struct { | |
| ctx client.Context | |
| fromName string | |
| digest string | |
| signMode apitxsigning.SignMode | |
| } | |
| tests := []struct { | |
| name string | |
| args args | |
| wantErr bool | |
| }{ | |
| { | |
| name: "signMode direct", | |
| args: args{ | |
| ctx: ctx, | |
| fromName: "direct", | |
| digest: "Hello world!", | |
| signMode: apitxsigning.SignMode_SIGN_MODE_DIRECT, | |
| }, | |
| wantErr: false, | |
| }, | |
| { | |
| name: "signMode textual", | |
| args: args{ | |
| ctx: ctx, | |
| fromName: "textual", | |
| digest: "Hello world!", | |
| signMode: apitxsigning.SignMode_SIGN_MODE_TEXTUAL, | |
| }, | |
| wantErr: false, | |
| }, | |
| { | |
| name: "signMode legacyAmino", | |
| args: args{ | |
| ctx: ctx, | |
| fromName: "legacy", | |
| digest: "Hello world!", | |
| signMode: apitxsigning.SignMode_SIGN_MODE_LEGACY_AMINO_JSON, | |
| }, | |
| wantErr: false, | |
| }, | |
| { | |
| name: "signMode direct aux", | |
| args: args{ | |
| ctx: ctx, | |
| fromName: "direct-aux", | |
| digest: "Hello world!", | |
| signMode: apitxsigning.SignMode_SIGN_MODE_DIRECT_AUX, | |
| }, | |
| wantErr: true, | |
| }, | |
| } | |
| for _, tt := range tests { | |
| t.Run(tt.name, func(t *testing.T) { | |
| _, err := k.NewAccount(tt.args.fromName, mnemonic, tt.name, "m/44'/118'/0'/0/0", hd.Secp256k1) | |
| require.NoError(t, err) | |
| got, err := sign(tt.args.ctx, tt.args.fromName, tt.args.digest, tt.args.signMode) | |
| if !tt.wantErr { | |
| require.NoError(t, err) | |
| require.NotNil(t, got) | |
| } else { | |
| require.Error(t, err) | |
| } | |
| }) | |
| } | |
| { | |
| name: "signMode direct aux", | |
| args: args{ | |
| ctx: ctx, | |
| fromName: "direct-aux", | |
| digest: "Hello world!", | |
| signMode: apitxsigning.SignMode_SIGN_MODE_DIRECT_AUX, | |
| }, | |
| wantErr: true, // Please add a comment explaining why an error is expected here. | |
| }, |
There was a problem hiding this comment.
mostly lgtm, but we need to add some docs in client/v2/offchain/README.md or client/v2/README.md under an offchain section. Otherwise, it is unclear how to use/discover the commands.
Personally, I have a small preference to add it directly in client/v2/README.md
And indend all the autocli docs one time more.
# Client/v2
## AutoCLI
## Offchain Signing
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@julienrbrt Readme added alongside output file flag and hex encoding |
Description
Closes:
#18483
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...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers 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...