Skip to content

Commit 3400229

Browse files
stevenjsaibatizoku
stevenj
and
saibatizoku
authored
fix(catalyst-toolbox): Fix error when purpose is null. | NPG-0000 (#534)
Catalyst toolbox was crashing if "purpose" was Null in a registration. This fixes that, so Null is properly detected as a "Catalyst" registration. Also, it adds the pre-calculated Jormungandr chain address, because its non trivial to go from the voting key to the chain address used by Jormungandr. This lets me compare the SVE snapshot with the normal snapshot, and it shows the SVE snapshot has rounding errors which will negatively effect people with multiple registrations on the same key. Recommendation is for Audit, we publish the updated `snapshot` artifact, not the one we import into vit-ss. And then make a quick program to convert the `snapshot` artifact into a format needed for vit-ss for F10. The `utilities/snapshot-check/compare_snapshot.py` would need a couple of tweaks to be such a tool. --------- Co-authored-by: Joaquín Rosales <[email protected]>
1 parent c2f8299 commit 3400229

File tree

11 files changed

+210
-26
lines changed

11 files changed

+210
-26
lines changed

‎containers/event-db-migrations/entry.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ check_env_vars() {
5050
# Iterate over the array and check if each variable is set
5151
for var in "${env_vars[@]}"; do
5252
echo "Checking $var"
53-
if [ -z "${!var}" ]; then
53+
if [ -z "${!var:-}" ]; then
5454
echo ">>> Error: $var is required and not set."
5555
exit 1
5656
fi

‎src/catalyst-toolbox/catalyst-toolbox/src/bin/cli/snapshot/mod.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use chain_addr::Discrimination;
12
use clap::Parser;
23
use color_eyre::Report;
34
use jcli_lib::utils::{output_file::OutputFile, output_format::OutputFormat};
@@ -45,6 +46,10 @@ pub struct SnapshotCmd {
4546

4647
#[clap(flatten)]
4748
output_format: OutputFormat,
49+
50+
/// Discrimination to use for initial addresses
51+
#[clap(short, long, default_value = "production")]
52+
discrimination: Discrimination,
4853
}
4954

5055
impl SnapshotCmd {
@@ -67,6 +72,7 @@ impl SnapshotCmd {
6772
self.min_stake_threshold,
6873
self.voting_power_cap,
6974
&assigner,
75+
self.discrimination,
7076
)?
7177
.to_full_snapshot_info();
7278
let mut out_writer = self.output.open()?;

‎src/catalyst-toolbox/catalyst-toolbox/src/rewards/voters.rs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ mod tests {
150150
DEFAULT_SNAPSHOT_THRESHOLD.into(),
151151
Fraction::from(1),
152152
&|_vk: &Identifier| String::new(),
153+
Discrimination::Production,
153154
)
154155
.unwrap();
155156

@@ -182,6 +183,7 @@ mod tests {
182183
DEFAULT_SNAPSHOT_THRESHOLD.into(),
183184
Fraction::from(1),
184185
&|_vk: &Identifier| String::new(),
186+
Discrimination::Production,
185187
)
186188
.unwrap();
187189

@@ -205,6 +207,7 @@ mod tests {
205207
DEFAULT_SNAPSHOT_THRESHOLD.into(),
206208
Fraction::from(1),
207209
&|_vk: &Identifier| String::new(),
210+
Discrimination::Production,
208211
)
209212
.unwrap();
210213

@@ -307,7 +310,7 @@ mod tests {
307310
voting_power: i.into(),
308311
reward_address,
309312
delegations,
310-
voting_purpose: 0,
313+
voting_purpose: Some(0),
311314
nonce: 0,
312315
});
313316
total_stake += i;
@@ -318,6 +321,7 @@ mod tests {
318321
0.into(),
319322
Fraction::from(1u64),
320323
&|_voting_key: &Identifier| String::new(),
324+
Discrimination::Production,
321325
)
322326
.unwrap();
323327

@@ -355,7 +359,7 @@ mod tests {
355359
voting_power: i.into(),
356360
reward_address,
357361
delegations,
358-
voting_purpose: 0,
362+
voting_purpose: Some(0),
359363
nonce: 0,
360364
});
361365
}
@@ -365,6 +369,7 @@ mod tests {
365369
0.into(),
366370
Fraction::new(1u64, 9u64),
367371
&|_vk: &Identifier| String::new(),
372+
Discrimination::Production,
368373
)
369374
.unwrap();
370375

@@ -390,6 +395,7 @@ mod tests {
390395
DEFAULT_SNAPSHOT_THRESHOLD.into(),
391396
Fraction::from(1),
392397
&|_vk: &Identifier| String::new(),
398+
Discrimination::Production,
393399
)
394400
.unwrap();
395401

‎src/catalyst-toolbox/snapshot-lib/src/lib.rs‎

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use chain_addr::Discrimination;
12
pub use fraction::Fraction;
23
use jormungandr_lib::{crypto::account::Identifier, interfaces::Value};
34
use registration::{
@@ -107,6 +108,7 @@ impl Snapshot {
107108
stake_threshold: Value,
108109
cap: Fraction,
109110
voting_group_assigner: &impl VotingGroupAssigner,
111+
discrimination: Discrimination,
110112
) -> Result<Self, Error> {
111113
let raw_contribs = raw_snapshot
112114
.0
@@ -116,7 +118,10 @@ impl Snapshot {
116118
.filter(|reg| reg.voting_power >= std::cmp::max(stake_threshold, 1.into()))
117119
// TODO: add capability to select voting purpose for a snapshot.
118120
// At the moment Catalyst is the only one in use
119-
.filter(|reg| reg.voting_purpose == CATALYST_VOTING_PURPOSE_TAG)
121+
.filter(|reg| {
122+
reg.voting_purpose.unwrap_or(CATALYST_VOTING_PURPOSE_TAG)
123+
== CATALYST_VOTING_PURPOSE_TAG
124+
})
120125
.fold(BTreeMap::new(), |mut acc: BTreeMap<_, Vec<_>>, reg| {
121126
let VotingRegistration {
122127
reward_address,
@@ -170,7 +175,12 @@ impl Snapshot {
170175
.map(|(k, contributions)| SnapshotInfo {
171176
hir: VoterHIR {
172177
voting_group: voting_group_assigner.assign(&k),
173-
voting_key: k,
178+
voting_key: k.clone(),
179+
address: chain_addr::Address(
180+
discrimination,
181+
chain_addr::Kind::Account(k.to_inner().into()),
182+
)
183+
.into(),
174184
voting_power: contributions.iter().map(|c| c.value).sum::<u64>().into(),
175185
},
176186
contributions,
@@ -278,14 +288,16 @@ pub mod tests {
278288
_raw,
279289
_stake_threshold.into(),
280290
Fraction::from(1u64),
281-
&DummyAssigner
291+
&DummyAssigner,
292+
Discrimination::Production,
282293
)
283294
.unwrap()
284295
== Snapshot::from_raw_snapshot(
285296
add,
286297
_stake_threshold.into(),
287298
Fraction::from(1u64),
288-
&DummyAssigner
299+
&DummyAssigner,
300+
Discrimination::Production,
289301
)
290302
.unwrap(),
291303
_additional_reg.voting_power < _stake_threshold.into()
@@ -304,6 +316,7 @@ pub mod tests {
304316
threshold.into(),
305317
Fraction::from(1),
306318
&|_vk: &Identifier| String::new(),
319+
Discrimination::Production,
307320
)
308321
.unwrap()
309322
})
@@ -319,6 +332,7 @@ pub mod tests {
319332
0.into(),
320333
Fraction::from(1),
321334
&|_vk: &Identifier| String::new(),
335+
Discrimination::Production,
322336
)
323337
.unwrap();
324338
let total_stake = snapshot
@@ -331,20 +345,22 @@ pub mod tests {
331345

332346
#[proptest]
333347
fn test_non_catalyst_regs_are_ignored(mut _reg: VotingRegistration) {
334-
_reg.voting_purpose = 1;
348+
_reg.voting_purpose = Some(1);
335349
assert_eq!(
336350
Snapshot::from_raw_snapshot(
337351
vec![_reg].into(),
338352
0.into(),
339353
Fraction::from(1u64),
340-
&DummyAssigner
354+
&DummyAssigner,
355+
Discrimination::Production,
341356
)
342357
.unwrap(),
343358
Snapshot::from_raw_snapshot(
344359
vec![].into(),
345360
0.into(),
346361
Fraction::from(1u64),
347-
&DummyAssigner
362+
&DummyAssigner,
363+
Discrimination::Production,
348364
)
349365
.unwrap(),
350366
)
@@ -367,7 +383,7 @@ pub mod tests {
367383
voting_power: i.into(),
368384
reward_address: RewardAddress(String::new()),
369385
delegations,
370-
voting_purpose: 0,
386+
voting_purpose: Some(0),
371387
nonce: 0,
372388
});
373389
}
@@ -377,6 +393,7 @@ pub mod tests {
377393
0.into(),
378394
Fraction::from(1u64),
379395
&DummyAssigner,
396+
Discrimination::Production,
380397
)
381398
.unwrap();
382399
let vp_1: u64 = snapshot
@@ -408,9 +425,14 @@ pub mod tests {
408425
}
409426
]"#,
410427
).unwrap();
411-
let snapshot =
412-
Snapshot::from_raw_snapshot(raw, 0.into(), Fraction::from(1u64), &DummyAssigner)
413-
.unwrap();
428+
let snapshot = Snapshot::from_raw_snapshot(
429+
raw,
430+
0.into(),
431+
Fraction::from(1u64),
432+
&DummyAssigner,
433+
Discrimination::Production,
434+
)
435+
.unwrap();
414436
assert_eq!(
415437
snapshot.contributions_for_voting_key(
416438
Identifier::from_hex(

‎src/catalyst-toolbox/snapshot-lib/src/registration.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub struct VotingRegistration {
5555
pub delegations: Delegations,
5656
/// 0 = Catalyst, assumed 0 for old legacy registrations
5757
#[serde(default)]
58-
pub voting_purpose: u64,
58+
pub voting_purpose: Option<u64>,
5959

6060
#[serde(default)]
6161
pub nonce: u64,
@@ -341,7 +341,7 @@ mod tests {
341341
voting_power,
342342
reward_address,
343343
delegations,
344-
voting_purpose: 0,
344+
voting_purpose: None,
345345
nonce: 0,
346346
}
347347
})

‎src/catalyst-toolbox/snapshot-lib/src/voter_hir.rs‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use ::serde::{Deserialize, Serialize};
2-
use jormungandr_lib::{crypto::account::Identifier, interfaces::Value};
2+
use jormungandr_lib::{crypto::account::Identifier, interfaces::Address, interfaces::Value};
33

44
pub type VotingGroup = String;
55

@@ -17,6 +17,9 @@ pub struct VoterHIR {
1717
// Keep hex encoding as in CIP-36
1818
#[serde(with = "serde")]
1919
pub voting_key: Identifier,
20+
// Jormungandr chain address
21+
pub address: Address,
22+
2023
/// Voting group this key belongs to.
2124
/// If this key belong to multiple voting groups, multiple records for the same
2225
/// key will be used.
@@ -59,7 +62,18 @@ pub mod tests {
5962
fn arbitrary_with(args: Self::Parameters) -> Self::Strategy {
6063
(any::<[u8; 32]>(), args.1 .0)
6164
.prop_map(move |(key, voting_power)| VoterHIR {
65+
// Two representations of exactly the same key.
6266
voting_key: Identifier::from_hex(&hex::encode(key)).unwrap(),
67+
address: chain_addr::Address(
68+
chain_addr::Discrimination::Production,
69+
chain_addr::Kind::Account(
70+
Identifier::from_hex(&hex::encode(key))
71+
.unwrap()
72+
.to_inner()
73+
.into(),
74+
),
75+
)
76+
.into(),
6377
voting_power: voting_power.into(),
6478
voting_group: args.0.clone(),
6579
})

‎src/vit-servicing-station/vit-servicing-station-tests/src/common/raw_snapshot.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use std::convert::TryInto;
22

3+
use chain_addr::Discrimination;
34
use chain_impl_mockchain::testing::TestGen;
45
use jormungandr_lib::interfaces::Value;
56
use rand::Rng;
@@ -60,6 +61,7 @@ impl RawSnapshotExtension for RawSnapshot {
6061
self.content.min_stake_threshold,
6162
self.content.voting_power_cap,
6263
assigner,
64+
Discrimination::Production,
6365
)?
6466
.to_full_snapshot_info())
6567
}
@@ -200,7 +202,7 @@ impl RawSnapshotBuilder {
200202
delegation_type_count += 1;
201203
Delegations::Legacy(TestGen::identifier().into())
202204
},
203-
voting_purpose: CATALYST_VOTING_PURPOSE_TAG,
205+
voting_purpose: Some(CATALYST_VOTING_PURPOSE_TAG),
204206
nonce: 0,
205207
})
206208
})

‎src/vit-servicing-station/vit-servicing-station-tests/src/common/snapshot.rs‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,20 @@ impl SnapshotBuilder {
100100
})
101101
.take(self.contributions_count)
102102
.collect(),
103-
hir: VoterHIR {
104-
voting_key: TestGen::identifier().into(),
105-
voting_group: self.groups[rng.gen_range(0, self.groups.len())]
106-
.to_string(),
107-
voting_power: rng.gen_range(1u64, 1_000u64).into(),
103+
hir: {
104+
let identifier = TestGen::identifier();
105+
106+
VoterHIR {
107+
voting_key: identifier.clone().into(),
108+
voting_group: self.groups[rng.gen_range(0, self.groups.len())]
109+
.to_string(),
110+
voting_power: rng.gen_range(1u64, 1_000u64).into(),
111+
address: chain_addr::Address(
112+
chain_addr::Discrimination::Production,
113+
chain_addr::Kind::Account(identifier.into()),
114+
)
115+
.into(),
116+
}
108117
},
109118
})
110119
})

‎src/vit-testing/mainnet-tools/src/snapshot/convert.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ impl OutputExtension for SnapshotEntry {
130130
VotingDelegations::New(new)
131131
}
132132
},
133-
voting_purpose: *self.voting_purpose.unwrap_or(VotingPurpose::CATALYST),
133+
voting_purpose: Some(*self.voting_purpose.unwrap_or(VotingPurpose::CATALYST)),
134134

135135
nonce: self.nonce,
136136
})

0 commit comments

Comments
 (0)