Skip to content

Commit 549868e

Browse files
cyrossignolcyrossignol
committed
Serialize beacon public key for self-service deletion
This removes the condition that omits a beacon public key from a beacon deletion contract so that validation passes for self-service removal.
1 parent 0d0ee04 commit 549868e

File tree

3 files changed

+57
-63
lines changed

3 files changed

+57
-63
lines changed

‎src/neuralnet/beacon.cpp‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -167,12 +167,18 @@ BeaconPayload::BeaconPayload()
167167
{
168168
}
169169

170-
BeaconPayload::BeaconPayload(const Cpid cpid, Beacon beacon)
171-
: m_cpid(cpid)
170+
BeaconPayload::BeaconPayload(const uint32_t version, const Cpid cpid, Beacon beacon)
171+
: m_version(version)
172+
, m_cpid(cpid)
172173
, m_beacon(std::move(beacon))
173174
{
174175
}
175176

177+
BeaconPayload::BeaconPayload(const Cpid cpid, Beacon beacon)
178+
: BeaconPayload(CURRENT_VERSION, cpid, std::move(beacon))
179+
{
180+
}
181+
176182
BeaconPayload BeaconPayload::Parse(const std::string& key, const std::string& value)
177183
{
178184
const CpidOption cpid = MiningId::Parse(key).TryCpid();
@@ -181,13 +187,8 @@ BeaconPayload BeaconPayload::Parse(const std::string& key, const std::string& va
181187
return BeaconPayload();
182188
}
183189

184-
Beacon beacon = Beacon::Parse(value);
185-
186-
if (!beacon.WellFormed()) {
187-
return BeaconPayload();
188-
}
189-
190-
return BeaconPayload(*cpid, std::move(beacon));
190+
// Legacy beacon payloads always parse to version 1:
191+
return BeaconPayload(1, *cpid, Beacon::Parse(value));
191192
}
192193

193194
bool BeaconPayload::Sign(CKey& private_key)

‎src/neuralnet/beacon.h‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,15 @@ class BeaconPayload : public IContractPayload
217217
//!
218218
BeaconPayload();
219219

220+
//!
221+
//! \brief Initialize a beacon payload for the specified CPID.
222+
//!
223+
//! \param version Version of the serialized beacon format.
224+
//! \param cpid Identifies the researcher that advertised the beacon.
225+
//! \param beacon Contains the beacon public key.
226+
//!
227+
BeaconPayload(const uint32_t version, const Cpid cpid, Beacon beacon);
228+
220229
//!
221230
//! \brief Initialize a beacon payload for the specified CPID.
222231
//!
@@ -252,15 +261,20 @@ class BeaconPayload : public IContractPayload
252261
//!
253262
bool WellFormed(const ContractAction action) const override
254263
{
255-
return m_version > 0
256-
&& m_version <= CURRENT_VERSION
257-
&& (action == ContractAction::REMOVE || m_beacon.WellFormed())
264+
if (m_version <= 0 || m_version > CURRENT_VERSION) {
265+
return false;
266+
}
267+
268+
if (m_version == 1) {
269+
return m_beacon.WellFormed() || action == ContractAction::REMOVE;
270+
}
271+
272+
return m_beacon.WellFormed()
258273
// The DER-encoded ASN.1 ECDSA signatures typically contain 70 or
259274
// 71 bytes, but may hold up to 73. Sizes as low as 68 bytes seen
260275
// on mainnet. We only check the number of bytes here as an early
261276
// step:
262-
&& (m_version == 1
263-
|| (m_signature.size() >= 64 && m_signature.size() <= 73));
277+
&& (m_signature.size() >= 64 && m_signature.size() <= 73);
264278
}
265279

266280
//!
@@ -316,10 +330,7 @@ class BeaconPayload : public IContractPayload
316330
{
317331
READWRITE(m_version);
318332
READWRITE(m_cpid);
319-
320-
if (contract_action != ContractAction::REMOVE) {
321-
READWRITE(m_beacon);
322-
}
333+
READWRITE(m_beacon);
323334

324335
if (!(s.GetType() & SER_GETHASH)) {
325336
READWRITE(m_signature);

‎src/test/neuralnet/beacon_tests.cpp‎

Lines changed: 27 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,8 @@ BOOST_AUTO_TEST_CASE(it_parses_a_payload_from_a_legacy_contract_key_and_value)
255255

256256
const NN::BeaconPayload payload = NN::BeaconPayload::Parse(key, value);
257257

258-
BOOST_CHECK_EQUAL(payload.m_version, NN::BeaconPayload::CURRENT_VERSION);
258+
// Legacy beacon payloads always parse to version 1:
259+
BOOST_CHECK_EQUAL(payload.m_version, 1);
259260
BOOST_CHECK(payload.m_cpid == cpid);
260261
BOOST_CHECK(payload.m_beacon.m_public_key == TestKey::Public());
261262
BOOST_CHECK_EQUAL(payload.m_beacon.m_timestamp, 0);
@@ -274,33 +275,44 @@ BOOST_AUTO_TEST_CASE(it_behaves_like_a_contract_payload)
274275
BOOST_CHECK(payload.RequiredBurnAmount() > 0);
275276
}
276277

277-
BOOST_AUTO_TEST_CASE(it_checks_whether_the_payload_is_well_formed_for_add)
278+
BOOST_AUTO_TEST_CASE(it_checks_whether_the_payload_is_well_formed)
278279
{
279280
const NN::Cpid cpid = NN::Cpid::Parse("00010203040506070809101112131415");
280281
NN::BeaconPayload valid(cpid, NN::Beacon(TestKey::Public()));
281282
valid.m_signature = TestKey::Signature();
282283

283284
BOOST_CHECK(valid.WellFormed(NN::ContractAction::ADD) == true);
285+
BOOST_CHECK(valid.WellFormed(NN::ContractAction::REMOVE) == true);
284286

285287
NN::BeaconPayload zero_cpid{NN::Cpid(), NN::Beacon(TestKey::Public())};
286288
zero_cpid.m_signature = TestKey::Signature();
287289

288290
// A zero CPID is technically valid...
289291
BOOST_CHECK(zero_cpid.WellFormed(NN::ContractAction::ADD) == true);
292+
BOOST_CHECK(zero_cpid.WellFormed(NN::ContractAction::REMOVE) == true);
290293

291294
NN::BeaconPayload missing_key(cpid, NN::Beacon());
292295
missing_key.m_signature = TestKey::Signature();
293296

294297
BOOST_CHECK(missing_key.WellFormed(NN::ContractAction::ADD) == false);
298+
BOOST_CHECK(missing_key.WellFormed(NN::ContractAction::REMOVE) == false);
295299
}
296300

297-
BOOST_AUTO_TEST_CASE(it_checks_whether_the_payload_is_well_formed_for_delete)
301+
BOOST_AUTO_TEST_CASE(it_checks_whether_a_legacy_v1_payload_is_well_formed)
298302
{
299303
const NN::Cpid cpid = NN::Cpid::Parse("00010203040506070809101112131415");
300-
NN::BeaconPayload valid(cpid, NN::Beacon());
301-
valid.m_signature = TestKey::Signature();
304+
const NN::Beacon beacon(TestKey::Public());
302305

303-
BOOST_CHECK(valid.WellFormed(NN::ContractAction::REMOVE) == true);
306+
const NN::BeaconPayload add = NN::BeaconPayload(1, cpid, beacon);
307+
308+
BOOST_CHECK(add.WellFormed(NN::ContractAction::ADD) == true);
309+
// Legacy beacon deletion contracts ignore the value:
310+
BOOST_CHECK(add.WellFormed(NN::ContractAction::REMOVE) == true);
311+
312+
const NN::BeaconPayload remove = NN::BeaconPayload(1, cpid, NN::Beacon());
313+
314+
BOOST_CHECK(remove.WellFormed(NN::ContractAction::ADD) == false);
315+
BOOST_CHECK(remove.WellFormed(NN::ContractAction::REMOVE) == true);
304316
}
305317

306318
BOOST_AUTO_TEST_CASE(it_signs_the_payload)
@@ -335,7 +347,7 @@ BOOST_AUTO_TEST_CASE(it_verifies_the_payload_signature)
335347
BOOST_CHECK(payload.VerifySignature());
336348
}
337349

338-
BOOST_AUTO_TEST_CASE(it_serializes_to_a_stream_for_add)
350+
BOOST_AUTO_TEST_CASE(it_serializes_to_a_stream)
339351
{
340352
const NN::Cpid cpid = NN::Cpid::Parse("00010203040506070809101112131415");
341353
const NN::Beacon beacon(TestKey::Public());
@@ -358,20 +370,22 @@ BOOST_AUTO_TEST_CASE(it_serializes_to_a_stream_for_add)
358370
expected.end());
359371
}
360372

361-
BOOST_AUTO_TEST_CASE(it_deserializes_from_a_stream_for_add)
373+
BOOST_AUTO_TEST_CASE(it_deserializes_from_a_stream)
362374
{
363375
const NN::Cpid cpid = NN::Cpid::Parse("00010203040506070809101112131415");
364376
const NN::Beacon beacon(TestKey::Public());
365377
const std::vector<uint8_t> signature = TestKey::Signature();
366378

367-
CDataStream stream = CDataStream(SER_NETWORK, PROTOCOL_VERSION)
379+
CDataStream stream_add = CDataStream(SER_NETWORK, PROTOCOL_VERSION)
368380
<< NN::BeaconPayload::CURRENT_VERSION
369381
<< cpid
370382
<< beacon
371383
<< signature;
372384

385+
CDataStream stream_remove = stream_add;
386+
373387
NN::BeaconPayload payload;
374-
payload.Unserialize(stream, NN::ContractAction::ADD);
388+
payload.Unserialize(stream_add, NN::ContractAction::ADD);
375389

376390
BOOST_CHECK_EQUAL(payload.m_version, NN::BeaconPayload::CURRENT_VERSION);
377391
BOOST_CHECK(payload.m_cpid == cpid);
@@ -385,45 +399,13 @@ BOOST_AUTO_TEST_CASE(it_deserializes_from_a_stream_for_add)
385399
signature.end());
386400

387401
BOOST_CHECK(payload.WellFormed(NN::ContractAction::ADD) == true);
388-
}
389402

390-
BOOST_AUTO_TEST_CASE(it_serializes_to_a_stream_for_delete)
391-
{
392-
const NN::Cpid cpid = NN::Cpid::Parse("00010203040506070809101112131415");
393-
NN::BeaconPayload payload(cpid, NN::Beacon());
394-
payload.m_signature = TestKey::Signature();
395-
396-
const CDataStream expected = CDataStream(SER_NETWORK, PROTOCOL_VERSION)
397-
<< NN::BeaconPayload::CURRENT_VERSION
398-
<< cpid
399-
<< payload.m_signature;
400-
401-
CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
402-
payload.Serialize(stream, NN::ContractAction::REMOVE);
403-
404-
BOOST_CHECK_EQUAL_COLLECTIONS(
405-
stream.begin(),
406-
stream.end(),
407-
expected.begin(),
408-
expected.end());
409-
}
410-
411-
BOOST_AUTO_TEST_CASE(it_deserializes_from_a_stream_for_delete)
412-
{
413-
const NN::Cpid cpid = NN::Cpid::Parse("00010203040506070809101112131415");
414-
const std::vector<uint8_t> signature = TestKey::Signature();
415-
416-
CDataStream stream = CDataStream(SER_NETWORK, PROTOCOL_VERSION)
417-
<< NN::BeaconPayload::CURRENT_VERSION
418-
<< cpid
419-
<< signature;
420-
421-
NN::BeaconPayload payload;
422-
payload.Unserialize(stream, NN::ContractAction::REMOVE);
403+
payload = NN::BeaconPayload();
404+
payload.Unserialize(stream_remove, NN::ContractAction::REMOVE);
423405

424406
BOOST_CHECK_EQUAL(payload.m_version, NN::BeaconPayload::CURRENT_VERSION);
425407
BOOST_CHECK(payload.m_cpid == cpid);
426-
BOOST_CHECK(payload.m_beacon.m_public_key.Raw().empty() == true);
408+
BOOST_CHECK(payload.m_beacon.m_public_key == TestKey::Public());
427409
BOOST_CHECK_EQUAL(payload.m_beacon.m_timestamp, 0);
428410

429411
BOOST_CHECK_EQUAL_COLLECTIONS(

0 commit comments

Comments
 (0)