search

anoma / namada

Rust implementation of Namada, a Proof-of-Stake L1 for interchain asset-agnostic privacy
https://namada.net
GNU General Public License v3.0
2.4k stars 953 forks source link

Some Byzantine validators may not be getting slashed #200

Closed sug0 closed 2 years ago

sug0 commented 2 years ago

I believe the PosBase API is being misused in the Shell.

This trait contains a method read_validator_address_raw_hash, which takes a raw_hash parameter of type impl AsRef<str> (the Tendermint address of some validator), and returns the Address of the equivalent node on Anoma's side, from what I've gathered.

ABCI++ hands us an Evidence struct for each Byzantine validator detected during a consensus round, which internally contains a Validator struct reflecting the address of the Byzantine node. This address is the 20-byte prefix of the SHA256 hash of the public key of a Tendermint validator.

When processing individual Evidence structs, we try to decode the address handed us via ABCI++ as a UTF-8 string, which may not always succeed (since we are dealing with a raw hash). In case this operation fails, we skip Byzantine validators which would have otherwise been slashed.

tzemanovic commented 2 years ago

nice catch! I'm not sure if it's currently broken because we also give tendermint validator's address, which isn't derived from consensus key and it seems to be fine with it, but it's not following its convention, so I think it really should be changed to use the consensus key instead.

Let's check this with #14.