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

feat: fix equivocation e2e-test + fix CLI #2248

Merged
merged 10 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 145 additions & 9 deletions tests/e2e/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -2862,26 +2862,162 @@ func (tr Chain) GetPathNameForGorelayer(chainA, chainB ChainID) string {
return pathName
}

// Run an instance of the Hermes relayer using the "evidence" command,
// which detects evidences committed to the blocks of a consumer chain.
// detect evidences committed to the blocks of a consumer chain
// either by running an instance of the Hermes relayer using the "evidence" command,
// or by queyring manually the consumer chain.
// Each infraction detected is reported to the provider chain using
// either a SubmitConsumerDoubleVoting or a SubmitConsumerMisbehaviour message.
type StartConsumerEvidenceDetectorAction struct {
Chain ChainID
Chain ChainID
Submitter ValidatorID
}

func (tr Chain) startConsumerEvidenceDetector(
action StartConsumerEvidenceDetectorAction,
useRelayer bool,
verbose bool,
) {
chainConfig := tr.testConfig.chainConfigs[action.Chain]
// run in detached mode so it will keep running in the background
bz, err := tr.target.ExecDetachedCommand(
"hermes", "evidence", "--chain", string(chainConfig.ChainId)).CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
// the Hermes doesn't support evidence handling for Permissionless ICS yet
// TODO: @Simon refactor once https://github.com/informalsystems/hermes/pull/4182 is merged.
if useRelayer {
// run in detached mode so it will keep running in the background
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understood correctly, it seems that if !useRelayer, we do not run this in the background but check if there's currently evidence. If so, it might make sense to rename startConsumerEvidenceDetector to something like detectEvidence.... If that's the case, maybe we should run Hermes in a similar way here and not in detached mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! Renamed to detectConsumerEvidenceAction

bz, err := tr.target.ExecDetachedCommand(
"hermes", "evidence", "--chain", string(chainConfig.ChainId)).CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}
tr.waitBlocks("provi", 10, 2*time.Minute)
} else {
// detect and the evidence on the consumer chain
sainoe marked this conversation as resolved.
Show resolved Hide resolved
consumerBinaryName := tr.testConfig.chainConfigs[action.Chain].BinaryName

// get the infraction height by querying the SDK evidence module of the consumer
timeout := time.Now().Add(30 * time.Second)
infractionHeight := int64(0)
for {
cmd := tr.target.ExecCommand(
consumerBinaryName,
"query", "evidence", "list",
`--node`, tr.target.GetQueryNode(action.Chain),
`-o`, `json`,
)

if verbose {
fmt.Println("query evidence cmd:", cmd.String())
}

bz, err := cmd.CombinedOutput()
if err == nil {
evidence := gjson.Get(string(bz), "evidence")
if len(evidence.Array()) > 0 {
infractionHeight = evidence.Array()[0].Get("value.height").Int()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could it be that the one we're looking for is not the first entry in the list?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We expect only one evidence. I'll update the condition above to reflect this!

break
}
}

if err != nil || time.Now().After(timeout) {
log.Print("Failed running command: ", cmd)
log.Fatal(err, "\n", string(bz))
}
time.Sleep(2 * time.Second)
}

// get the evidence data from the block
// note that the the evidence is added to the next block after the infraction height
sainoe marked this conversation as resolved.
Show resolved Hide resolved
cmd := tr.target.ExecCommand(
consumerBinaryName,
"query", "block", "--type=height", strconv.Itoa(int(infractionHeight+1)),
`--node`, tr.target.GetQueryNode(action.Chain),
`-o`, `json`,
)

if verbose {
fmt.Println("query block for evidence cmd:", cmd.String())
}

bz, err := cmd.CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}

evidence := gjson.Get(string(bz), "evidence.evidence").Array()
if len(evidence) == 0 {
log.Fatal("expected at least one evidence in block but found zero")
}

if equivocation := evidence[0].Get("duplicate_vote_evidence"); equivocation.String() != "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also here, should we iterate over the list until we find a "duplicate_vote_evidence"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could but it's simpler to return an error if the evidence isn't of the duplicate vote evidence type since we expect only one evidence.

// persist evidence in the json format
evidenceJson := equivocation.Raw
evidencePath := "/temp-evidence.json"
bz, err = tr.target.ExecCommand(
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, evidenceJson, evidencePath),
).CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}

// query IBC header at the infraction height
cmd = tr.target.ExecCommand(
consumerBinaryName,
"query", "ibc", "client", "header", "--height", strconv.Itoa(int(infractionHeight)),
`--node`, tr.target.GetQueryNode(action.Chain),
`-o`, `json`,
)

if verbose {
fmt.Println("query IBC header cmd:", cmd.String())
}

bz, err = cmd.CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}

// persist IBC header in json format
headerPath := "/temp-header.json"
bz, err = tr.target.ExecCommand(
"/bin/bash", "-c", fmt.Sprintf(`echo '%s' > %s`, string(bz), headerPath),
).CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}

// submit consumer equivocation to provider
gas := "auto"
submitEquivocation := fmt.Sprintf(
`%s tx provider submit-consumer-double-voting %s %s %s --from validator%s --chain-id %s --home %s --node %s --gas %s --keyring-backend test -y -o json`,
tr.testConfig.chainConfigs[ChainID("provi")].BinaryName,
string(tr.testConfig.chainConfigs[action.Chain].ConsumerId),
evidencePath,
headerPath,
action.Submitter,
tr.testConfig.chainConfigs[ChainID("provi")].ChainId,
tr.getValidatorHome(ChainID("provi"), action.Submitter),
tr.getValidatorNode(ChainID("provi"), action.Submitter),
gas,
)

cmd = tr.target.ExecCommand(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a bash command?

Copy link
Contributor Author

@sainoe sainoe Sep 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find it easier to execute a long command that way.

"/bin/bash", "-c",
submitEquivocation,
)

if verbose {
fmt.Println("submit consumer equivocation cmd:", cmd.String())
}

bz, err = cmd.CombinedOutput()
if err != nil {
log.Fatal(err, "\n", string(bz))
}
}

// TODO: @sainoe handle misbehaviour
// else if misb := evidence[0].Get("light_client_attack_evidence"); misb.String() != "" {
//}
tr.waitBlocks("provi", 3, 1*time.Minute)
}
tr.waitBlocks("provi", 10, 2*time.Minute)
}

type OptInAction struct {
Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/steps_double_sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ func stepsCauseDoubleSignOnConsumer(consumerName, providerName string) []Step {
// and jail and slashing of bob on the provider
{
Action: StartConsumerEvidenceDetectorAction{
Chain: ChainID(consumerName),
Chain: ChainID(consumerName),
Submitter: ValidatorID("bob"),
},
State: State{
ChainID(providerName): ChainState{
Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/test_driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func (td *DefaultDriver) runAction(action interface{}) error {
case UpdateLightClientAction:
target.updateLightClient(action, td.verbose)
case StartConsumerEvidenceDetectorAction:
target.startConsumerEvidenceDetector(action, td.verbose)
target.startConsumerEvidenceDetector(action, false, td.verbose)
case SubmitChangeRewardDenomsProposalAction:
target = td.getTargetDriver(action.Chain)
version := target.testConfig.providerVersion
Expand Down
11 changes: 5 additions & 6 deletions x/ccv/provider/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,24 +170,23 @@ Example:
txf = txf.WithTxConfig(clientCtx.TxConfig).WithAccountRetriever(clientCtx.AccountRetriever)

submitter := clientCtx.GetFromAddress()
cdc := codec.NewProtoCodec(clientCtx.InterfaceRegistry)

ev := tmproto.DuplicateVoteEvidence{}
evidenceJson, err := os.ReadFile(args[0])
evidenceJson, err := os.ReadFile(args[1])
if err != nil {
return err
}

if err := json.Unmarshal(evidenceJson, &ev); err != nil {
ev := tmproto.DuplicateVoteEvidence{}
if err := cdc.UnmarshalJSON(evidenceJson, &ev); err != nil {
return fmt.Errorf("duplicate vote evidence unmarshalling failed: %s", err)
}

headerJson, err := os.ReadFile(args[1])
headerJson, err := os.ReadFile(args[2])
if err != nil {
return err
}

cdc := codec.NewProtoCodec(clientCtx.InterfaceRegistry)

header := ibctmtypes.Header{}
if err := cdc.UnmarshalJSON(headerJson, &header); err != nil {
return fmt.Errorf("infraction IBC header unmarshalling failed: %s", err)
Expand Down
Loading