Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
3345: fix(ckb cli) add h256 validator for --assume-valid-target and ba-code-hash r=quake a=chanhsu001

fix(ckb cli) add h256 validator for --assume-valid-target and ba-code-hash

### What problem does this PR solve?

- add H256 validator for `ckb init --ba-code-hash` and `ckb run --assume-valid-target` replace naive is_hex check.
- add related 1 testcase

### What is changed and how it works?

now cli report error at parsing phase, prompt invalid h256 message, instead of running error.

### Related changes

- PR to update `owner/repo`:

Tests <!-- At least one of them must be included. -->

- Unit test
- Integration test

### Release note <!-- Choose from None, Title Only and Note. Bugfixes or new features need a release note. -->

```release-note
Title Only: Include only the PR title in the release note.
```



Co-authored-by: chanhsu <[email protected]>
  • Loading branch information
bors[bot] and chanhsu001 authored Mar 23, 2022
2 parents 45a7514 + e919605 commit bba8546
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 14 deletions.
12 changes: 10 additions & 2 deletions util/app-config/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ fn run<'help>() -> Command<'help> {
Arg::new(ARG_ASSUME_VALID_TARGET)
.long(ARG_ASSUME_VALID_TARGET)
.takes_value(true)
.validator(is_hex)
.validator(is_h256)
.help("This parameter specifies the hash of a block. \
When the height does not reach this block's height, the execution of the script will be disabled, \
that is, skip verifying the script content. \
Expand Down Expand Up @@ -417,7 +417,7 @@ fn init<'help>() -> Command<'help> {
Arg::new(ARG_BA_CODE_HASH)
.long(ARG_BA_CODE_HASH)
.value_name("code_hash")
.validator(is_hex)
.validator(is_h256)
.takes_value(true)
.help(
"Sets code_hash in [block_assembler] \
Expand Down Expand Up @@ -526,3 +526,11 @@ fn is_hex(hex: &str) -> Result<(), String> {
Err("Must 0x-prefixed hexadecimal string".to_string())
}
}

fn is_h256(hex: &str) -> Result<(), String> {
if hex.len() != 66 {
Err("Must be 0x-prefixed hexadecimal string and string length is 66".to_owned())
} else {
is_hex(hex)
}
}
71 changes: 59 additions & 12 deletions util/app-config/src/tests/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ fn ba_message_requires_ba_arg_or_ba_code_hash() {
"--ba-arg",
"0x00",
]);
let ok_ba_code_hash = basic_app().try_get_matches_from(&[
let ba_code_hash = basic_app().try_get_matches_from(&[
BIN_NAME,
"init",
"--ba-message",
Expand All @@ -52,11 +52,7 @@ fn ba_message_requires_ba_arg_or_ba_code_hash() {
"--ba-message is ok with --ba-arg, but gets error: {:?}",
ok_ba_arg.err()
);
assert!(
ok_ba_code_hash.is_ok(),
"--ba-message is ok with --ba-code-hash, but gets error: {:?}",
ok_ba_code_hash.err()
);
assert!(ba_code_hash.is_err());
assert!(
err.is_err(),
"--ba-message requires --ba-arg or --ba-code-hash"
Expand All @@ -73,19 +69,15 @@ fn ba_message_requires_ba_arg_or_ba_code_hash() {

#[test]
fn ba_arg_and_ba_code_hash() {
let ok_matches = basic_app().try_get_matches_from(&[
let matches = basic_app().try_get_matches_from(&[
BIN_NAME,
"init",
"--ba-code-hash",
"0x00",
"--ba-arg",
"0x00",
]);
assert!(
ok_matches.is_ok(),
"--ba-code-hash is OK with --ba-arg, but gets error: {:?}",
ok_matches.err()
);
assert!(matches.is_err());
}

#[test]
Expand All @@ -97,3 +89,58 @@ fn ba_advanced() {

assert_eq!(1, sub_matches.occurrences_of(ARG_BA_ADVANCED));
}

#[test]
/// 2 cases in which use h256 validator:
/// ckb init --ba-code-hash
/// ckb run --assume-valid-target
/// not for `ckb init --ba-arg` && `ckb init --ba-message`
fn h256_as_validator() {
let ok_matches = basic_app().try_get_matches_from(&[
BIN_NAME,
"init",
"--ba-code-hash",
"0x00d1b86f6824d33a91b72ec20e2118cf7788a5ffff656bd1ea1ea638c764cb5f",
"--ba-arg",
"0x00",
]);
assert!(ok_matches.is_ok());

let err_matches = basic_app().try_get_matches_from(&[
BIN_NAME,
"init",
"--ba-code-hash",
"0xd1b86f6824d33a91b72ec20e2118cf7788a5ffff656bd1ea1ea638c764cb5f",
"--ba-arg",
"0x00",
]);
let err = err_matches.err().unwrap();
assert_eq!(clap::ErrorKind::ValueValidation, err.kind());

let err_matches = basic_app().try_get_matches_from(&[
BIN_NAME,
"init",
"--ba-code-hash",
"0x4630c0",
"--ba-arg",
"0x00",
]);
let err = err_matches.err().unwrap();
assert_eq!(clap::ErrorKind::ValueValidation, err.kind());

let ok_matches = basic_app().try_get_matches_from(&[
BIN_NAME,
"run",
"--assume-valid-target",
"0x94a4e93601f7295501891764880d37e9fcf886d02bf64b3d06f9137db8fa981e",
]);
assert!(ok_matches.is_ok());
let err_matches = basic_app().try_get_matches_from(&[
BIN_NAME,
"run",
"--assume-valid-target",
"0x94a4e93601f729550",
]);
let err = err_matches.err().unwrap();
assert_eq!(clap::ErrorKind::ValueValidation, err.kind());
}

0 comments on commit bba8546

Please sign in to comment.