init validator tx - verify ownership of used keys #2163
Conversation
tzemanovic
force-pushed
the
tomas/init-validator-own-keys
branch
from
0f5b7ac to
a9771c2
Compare
|
The bench for init-validator is affected, it’s most likely missing the newly required signatures |
| tx: &Tx, | ||
| pks: Vec<common::PublicKey>, | ||
| ) -> EnvResult<bool> { | ||
| let max_signatures_per_transaction = |
There was a problem hiding this comment.
What if the max_signatures_per_transaction is lower than the number of validator keys at initialization?
There was a problem hiding this comment.
As we don't have support for validator multisig, it would only be an issue if the source of the tx is a mutisig with threshold > max_signatures_per_transaction - 5. We use 15 as the default which should be sufficient for either case
There was a problem hiding this comment.
ah, I was wrong on this - we do have multisig validator support, but with the default this would still allow up to 10 multisig participants which should be sufficient
| Ok(SigningTxData { | ||
| owner: None, | ||
| public_keys, | ||
| threshold: 0, |
There was a problem hiding this comment.
what does a threshold of 0 mean here?
There was a problem hiding this comment.
it's unused for signing - this field is only used for offline proposal votes
* tomas/init-validator-own-keys: changelog: add #2163 client: store validator protocol key in the regular wallet wasm/tx_init_validator: check ownership of used keys with sigs ledger: add a tx host fn for sig verification client: sign tx_init_validator with all keys used
* tomas/init-validator-own-keys: fix `init_validator` bench changelog: add #2163 client: store validator protocol key in the regular wallet wasm/tx_init_validator: check ownership of used keys with sigs ledger: add a tx host fn for sig verification client: sign tx_init_validator with all keys used
* tomas/init-validator-own-keys: fix `init_validator` bench changelog: add #2163 client: store validator protocol key in the regular wallet wasm/tx_init_validator: check ownership of used keys with sigs ledger: add a tx host fn for sig verification client: sign tx_init_validator with all keys used
shared/src/vm/host_env.rs
Outdated
| max_signatures, | ||
| // This uses VpGasMeter, so we're charging the gas here manually | ||
| // instead | ||
| &mut None, |
There was a problem hiding this comment.
Minor thing here, we can change the verify_signatures method of Tx to accept an &mut dyn GasMetering trait object just like we do in fetch_or_compile so that we can still charge gas for every single signature without the need to do it manually, if you prefer. Alternatively, since this function is only called by the init_validator tx, and we always require 4 valid signatures, we could also just skip charging gas in here since its cost is already included in the benchmark of the tx itself
There was a problem hiding this comment.
I've refactored it in 88578bf but without the dynamic dispatch using a lambda instead. It also moves the gas metering logic back into the relevant places in vm/host_env.rs instead of being done in proto/types.rs
* origin/tomas/init-validator-own-keys: refactor sig verification gas handling fix `init_validator` bench changelog: add #2163 client: store validator protocol key in the regular wallet wasm/tx_init_validator: check ownership of used keys with sigs ledger: add a tx host fn for sig verification client: sign tx_init_validator with all keys used # Conflicts: # apps/src/lib/client/tx.rs
* origin/tomas/init-validator-own-keys: refactor sig verification gas handling fix `init_validator` bench changelog: add #2163 client: store validator protocol key in the regular wallet wasm/tx_init_validator: check ownership of used keys with sigs ledger: add a tx host fn for sig verification client: sign tx_init_validator with all keys used # Conflicts: # apps/src/lib/client/tx.rs
* origin/tomas/init-validator-own-keys: refactor sig verification gas handling fix `init_validator` bench changelog: add #2163 client: store validator protocol key in the regular wallet wasm/tx_init_validator: check ownership of used keys with sigs ledger: add a tx host fn for sig verification client: sign tx_init_validator with all keys used # Conflicts: # apps/src/lib/client/tx.rs
* origin/tomas/init-validator-own-keys: refactor sig verification gas handling fix `init_validator` bench changelog: add #2163 client: store validator protocol key in the regular wallet wasm/tx_init_validator: check ownership of used keys with sigs ledger: add a tx host fn for sig verification client: sign tx_init_validator with all keys used # Conflicts: # apps/src/lib/client/tx.rs
Describe your changes
closes #106
Indicate on which release or other PRs this topic is based on
0.26.0
Checklist before merging to
draft