diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index e60c8cf37d3..c0ebe06dd7e 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -98,20 +98,22 @@ RCLConsensus::Adaptor::Adaptor( JLOG(j_.info()) << "Consensus engine started (cookie: " + std::to_string(valCookie_) + ")"; - if (validatorKeys_.nodeID != beast::zero) + if (validatorKeys_.nodeID != beast::zero && validatorKeys_.keys) { std::stringstream ss; JLOG(j_.info()) << "Validator identity: " << toBase58( TokenType::NodePublic, - validatorKeys_.masterPublicKey); + validatorKeys_.keys->masterPublicKey); - if (validatorKeys_.masterPublicKey != validatorKeys_.publicKey) + if (validatorKeys_.keys->masterPublicKey != + validatorKeys_.keys->publicKey) { JLOG(j_.debug()) << "Validator ephemeral signing key: " - << toBase58(TokenType::NodePublic, validatorKeys_.publicKey) + << toBase58( + TokenType::NodePublic, validatorKeys_.keys->publicKey) << " (seq: " << std::to_string(validatorKeys_.sequence) << ")"; } } @@ -210,13 +212,20 @@ RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) proposal.prevLedger().begin(), proposal.prevLedger().size()); prop.set_proposeseq(proposal.proposeSeq()); prop.set_closetime(proposal.closeTime().time_since_epoch().count()); - prop.set_nodepubkey( - validatorKeys_.publicKey.data(), validatorKeys_.publicKey.size()); - auto sig = signDigest( - validatorKeys_.publicKey, - validatorKeys_.secretKey, - proposal.signingHash()); + if (!validatorKeys_.keys) + { + JLOG(j_.warn()) << "RCLConsensus::Adaptor::propose: ValidatorKeys " + "not set: \n"; + return; + } + + auto const& keys = *validatorKeys_.keys; + + prop.set_nodepubkey(keys.publicKey.data(), keys.publicKey.size()); + + auto sig = + signDigest(keys.publicKey, keys.secretKey, proposal.signingHash()); prop.set_signature(sig.data(), sig.size()); @@ -225,7 +234,7 @@ RCLConsensus::Adaptor::propose(RCLCxPeerPos::Proposal const& proposal) proposal.prevLedger(), proposal.proposeSeq(), proposal.closeTime(), - validatorKeys_.publicKey, + keys.publicKey, sig); app_.getHashRouter().addSuppression(suppression); @@ -799,10 +808,19 @@ RCLConsensus::Adaptor::validate( validationTime = lastValidationTime_ + 1s; lastValidationTime_ = validationTime; + if (!validatorKeys_.keys) + { + JLOG(j_.warn()) << "RCLConsensus::Adaptor::validate: ValidatorKeys " + "not set\n"; + return; + } + + auto const& keys = *validatorKeys_.keys; + auto v = std::make_shared( lastValidationTime_, - validatorKeys_.publicKey, - validatorKeys_.secretKey, + keys.publicKey, + keys.secretKey, validatorKeys_.nodeID, [&](STValidation& v) { v.setFieldH256(sfLedgerHash, ledger.id()); @@ -960,7 +978,7 @@ RCLConsensus::Adaptor::preStartRound( { // We have a key, we do not want out of sync validations after a restart // and are not amendment blocked. - validating_ = validatorKeys_.publicKey.size() != 0 && + validating_ = validatorKeys_.keys && prevLgr.seq() >= app_.getMaxDisallowedLedger() && !app_.getOPs().isBlocked(); @@ -1033,7 +1051,7 @@ RCLConsensus::Adaptor::laggards( bool RCLConsensus::Adaptor::validator() const { - return !validatorKeys_.publicKey.empty(); + return validatorKeys_.keys.has_value(); } void diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 9aad155d876..55300a390c9 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -185,7 +185,7 @@ class ApplicationImp : public Application, public BasicApp NodeCache m_tempNodeCache; CachedSLEs cachedSLEs_; - std::pair nodeIdentity_; + std::optional> nodeIdentity_; ValidatorKeys const validatorKeys_; std::unique_ptr m_resourceManager; @@ -587,13 +587,20 @@ class ApplicationImp : public Application, public BasicApp std::pair const& nodeIdentity() override { - return nodeIdentity_; + if (nodeIdentity_) + return *nodeIdentity_; + + LogicError( + "Accessing Application::nodeIdentity() before it is initialized."); } - PublicKey const& + std::optional getValidationPublicKey() const override { - return validatorKeys_.publicKey; + if (!validatorKeys_.keys) + return {}; + + return validatorKeys_.keys->publicKey; } NetworkOPs& @@ -1345,7 +1352,7 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline) return false; } - if (validatorKeys_.publicKey.size()) + if (validatorKeys_.keys) setMaxDisallowedLedger(); // Configure the amendments the server supports @@ -1466,9 +1473,19 @@ ApplicationImp::setup(boost::program_options::variables_map const& cmdline) publisherManifests_->load(getWalletDB(), "PublisherManifests"); + // It is possible to have a valid ValidatorKeys object without + // setting the signingKey or masterKey. This occurs if the + // configuration file does not have either + // SECTION_VALIDATOR_TOKEN or SECTION_VALIDATION_SEED section. + + // masterKey for the configuration-file specified validator keys + std::optional localSigningKey; + if (validatorKeys_.keys) + localSigningKey = validatorKeys_.keys->publicKey; + // Setup trusted validators if (!validators_->load( - validatorKeys_.publicKey, + localSigningKey, config().section(SECTION_VALIDATORS).values(), config().section(SECTION_VALIDATOR_LIST_KEYS).values())) { diff --git a/src/ripple/app/main/Application.h b/src/ripple/app/main/Application.h index 67343852b66..3fa8d13e870 100644 --- a/src/ripple/app/main/Application.h +++ b/src/ripple/app/main/Application.h @@ -242,7 +242,7 @@ class Application : public beast::PropertyStream::Source virtual std::pair const& nodeIdentity() = 0; - virtual PublicKey const& + virtual std::optional getValidationPublicKey() const = 0; virtual Resource::Manager& diff --git a/src/ripple/app/misc/Manifest.h b/src/ripple/app/misc/Manifest.h index 88452e5e0cc..b1cb5d2f325 100644 --- a/src/ripple/app/misc/Manifest.h +++ b/src/ripple/app/misc/Manifest.h @@ -86,7 +86,10 @@ struct Manifest PublicKey masterKey; /// The ephemeral key associated with this manifest. - PublicKey signingKey; + // A revoked manifest does not have a signingKey + // This field is specified as "optional" in manifestFormat's + // SOTemplate + std::optional signingKey; /// The sequence number of this manifest. std::uint32_t sequence = 0; @@ -94,7 +97,22 @@ struct Manifest /// The domain, if one was specified in the manifest; empty otherwise. std::string domain; - Manifest() = default; + Manifest() = delete; + + Manifest( + std::string const& serialized_, + PublicKey const& masterKey_, + std::optional const& signingKey_, + std::uint32_t seq, + std::string const& domain_) + : serialized(serialized_) + , masterKey(masterKey_) + , signingKey(signingKey_) + , sequence(seq) + , domain(domain_) + { + } + Manifest(Manifest const& other) = delete; Manifest& operator=(Manifest const& other) = delete; @@ -110,6 +128,12 @@ struct Manifest uint256 hash() const; + /// Returns `true` if manifest revokes master key + // The maximum possible sequence number means that the master key has + // been revoked + static bool + revoked(std::uint32_t sequence); + /// Returns `true` if manifest revokes master key bool revoked() const; @@ -266,7 +290,7 @@ class ManifestCache May be called concurrently */ - PublicKey + std::optional getSigningKey(PublicKey const& pk) const; /** Returns ephemeral signing key's master public key. diff --git a/src/ripple/app/misc/NegativeUNLVote.cpp b/src/ripple/app/misc/NegativeUNLVote.cpp index aa9db60c33d..9b616be6ce1 100644 --- a/src/ripple/app/misc/NegativeUNLVote.cpp +++ b/src/ripple/app/misc/NegativeUNLVote.cpp @@ -90,7 +90,7 @@ NegativeUNLVote::doVoting( auto n = choose(prevLedger->info().hash, candidates.toDisableCandidates); assert(nidToKeyMap.count(n)); - addTx(seq, nidToKeyMap[n], ToDisable, initialSet); + addTx(seq, nidToKeyMap.at(n), ToDisable, initialSet); } if (!candidates.toReEnableCandidates.empty()) @@ -98,7 +98,7 @@ NegativeUNLVote::doVoting( auto n = choose( prevLedger->info().hash, candidates.toReEnableCandidates); assert(nidToKeyMap.count(n)); - addTx(seq, nidToKeyMap[n], ToReEnable, initialSet); + addTx(seq, nidToKeyMap.at(n), ToReEnable, initialSet); } } } diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 239fd6306e0..73912089f80 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1970,9 +1970,9 @@ NetworkOPsImp::pubManifest(Manifest const& mo) jvObj[jss::type] = "manifestReceived"; jvObj[jss::master_key] = toBase58(TokenType::NodePublic, mo.masterKey); - if (!mo.signingKey.empty()) + if (mo.signingKey) jvObj[jss::signing_key] = - toBase58(TokenType::NodePublic, mo.signingKey); + toBase58(TokenType::NodePublic, *mo.signingKey); jvObj[jss::seq] = Json::UInt(mo.sequence); if (auto sig = mo.getSignature()) jvObj[jss::signature] = strHex(*sig); @@ -2456,10 +2456,11 @@ NetworkOPsImp::getServerInfo(bool human, bool admin, bool counters) if (admin) { - if (!app_.getValidationPublicKey().empty()) + if (auto const localPubKey = app_.validators().localPublicKey(); + localPubKey && app_.getValidationPublicKey()) { - info[jss::pubkey_validator] = toBase58( - TokenType::NodePublic, app_.validators().localPublicKey()); + info[jss::pubkey_validator] = + toBase58(TokenType::NodePublic, localPubKey.value()); } else { diff --git a/src/ripple/app/misc/ValidatorKeys.h b/src/ripple/app/misc/ValidatorKeys.h index c58c95f8cc7..a6b53841739 100644 --- a/src/ripple/app/misc/ValidatorKeys.h +++ b/src/ripple/app/misc/ValidatorKeys.h @@ -36,13 +36,35 @@ class Config; class ValidatorKeys { public: - PublicKey masterPublicKey; - PublicKey publicKey; - SecretKey secretKey; + // Group all keys in a struct. Either all keys are valid or none are. + struct Keys + { + PublicKey masterPublicKey; + PublicKey publicKey; + SecretKey secretKey; + + Keys() = delete; + Keys( + PublicKey const& masterPublic_, + PublicKey const& public_, + SecretKey const& secret_) + : masterPublicKey(masterPublic_) + , publicKey(public_) + , secretKey(secret_) + { + } + }; + + // Note: The existence of keys cannot be used as a proxy for checking the + // validity of a configuration. It is possible to have a valid + // configuration while not setting the keys, as per the constructor of + // the ValidatorKeys class. + std::optional keys; NodeID nodeID; std::string manifest; std::uint32_t sequence = 0; + ValidatorKeys() = delete; ValidatorKeys(Config const& config, beast::Journal j); bool diff --git a/src/ripple/app/misc/ValidatorList.h b/src/ripple/app/misc/ValidatorList.h index 0d7605fc09d..280818abd35 100644 --- a/src/ripple/app/misc/ValidatorList.h +++ b/src/ripple/app/misc/ValidatorList.h @@ -247,7 +247,17 @@ class ValidatorList // a seed, the signing key is the same as the master key. hash_set trustedSigningKeys_; - PublicKey localPubKey_; + std::optional localPubKey_; + + // The below variable contains the Publisher list specified in the local + // config file under the title of SECTION_VALIDATORS or [validators]. + // This list is not associated with the masterKey of any publisher. + + // Appropos PublisherListCollection fields, localPublisherList does not + // have any "remaining" manifests. It is assumed to be perennially + // "available". The "validUntil" field is set to the highest possible + // value of the field, hence this list is always valid. + PublisherList localPublisherList; // The master public keys of the current negative UNL hash_set negativeUNL_; @@ -331,7 +341,7 @@ class ValidatorList */ bool load( - PublicKey const& localSigningKey, + std::optional const& localSigningKey, std::vector const& configKeys, std::vector const& publisherKeys); @@ -553,13 +563,14 @@ class ValidatorList bool trustedPublisher(PublicKey const& identity) const; - /** Returns local validator public key + /** This function returns the local validator public key + * or a std::nullopt @par Thread Safety May be called concurrently */ - PublicKey + std::optional localPublicKey() const; /** Invokes the callback once for every listed validation public key. @@ -766,6 +777,8 @@ class ValidatorList std::optional const& hash, lock_guard const&); + // This function updates the keyListings_ counts for all the trusted + // master keys void updatePublisherList( PublicKey const& pubKey, @@ -849,11 +862,10 @@ class ValidatorList Calling public member function is expected to lock mutex */ - ListDisposition + std::pair> verify( lock_guard const&, Json::Value& list, - PublicKey& pubKey, std::string const& manifest, std::string const& blob, std::string const& signature); diff --git a/src/ripple/app/misc/impl/Manifest.cpp b/src/ripple/app/misc/impl/Manifest.cpp index 60b52133053..2916d6d2f32 100644 --- a/src/ripple/app/misc/impl/Manifest.cpp +++ b/src/ripple/app/misc/impl/Manifest.cpp @@ -22,8 +22,6 @@ #include #include #include -#include -#include #include #include #include @@ -32,7 +30,6 @@ #include #include -#include #include namespace ripple { @@ -45,8 +42,11 @@ to_string(Manifest const& m) if (m.revoked()) return "Revocation Manifest " + mk; + if (!m.signingKey) + Throw("No SigningKey in manifest " + mk); + return "Manifest " + mk + " (" + std::to_string(m.sequence) + ": " + - toBase58(TokenType::NodePublic, m.signingKey) + ")"; + toBase58(TokenType::NodePublic, *m.signingKey) + ")"; } std::optional @@ -96,25 +96,27 @@ deserializeManifest(Slice s, beast::Journal journal) if (!publicKeyType(makeSlice(pk))) return std::nullopt; - Manifest m; - m.serialized.assign(reinterpret_cast(s.data()), s.size()); - m.masterKey = PublicKey(makeSlice(pk)); - m.sequence = st.getFieldU32(sfSequence); + PublicKey const masterKey = PublicKey(makeSlice(pk)); + std::uint32_t const seq = st.getFieldU32(sfSequence); + + std::string domain; + + std::optional signingKey; if (st.isFieldPresent(sfDomain)) { auto const d = st.getFieldVL(sfDomain); - m.domain.assign(reinterpret_cast(d.data()), d.size()); + domain.assign(reinterpret_cast(d.data()), d.size()); - if (!isProperlyFormedTomlDomain(m.domain)) + if (!isProperlyFormedTomlDomain(domain)) return std::nullopt; } bool const hasEphemeralKey = st.isFieldPresent(sfSigningPubKey); bool const hasEphemeralSig = st.isFieldPresent(sfSignature); - if (m.revoked()) + if (Manifest::revoked(seq)) { // Revocation manifests should not specify a new signing key // or a signing key signature. @@ -139,14 +141,18 @@ deserializeManifest(Slice s, beast::Journal journal) if (!publicKeyType(makeSlice(spk))) return std::nullopt; - m.signingKey = PublicKey(makeSlice(spk)); + signingKey.emplace(makeSlice(spk)); // The signing and master keys can't be the same - if (m.signingKey == m.masterKey) + if (*signingKey == masterKey) return std::nullopt; } - return m; + std::string const serialized( + reinterpret_cast(s.data()), s.size()); + + // If the manifest is revoked, then the signingKey will be unseated + return Manifest(serialized, masterKey, signingKey, seq, domain); } catch (std::exception const& ex) { @@ -192,9 +198,14 @@ Manifest::verify() const SerialIter sit(serialized.data(), serialized.size()); st.set(sit); + // The manifest must either have a signing key or be revoked. This check + // prevents us from accessing an unseated signingKey in the next check. + if (!revoked() && !signingKey) + return false; + // Signing key and signature are not required for // master key revocations - if (!revoked() && !ripple::verify(st, HashPrefix::manifest, signingKey)) + if (!revoked() && !ripple::verify(st, HashPrefix::manifest, *signingKey)) return false; return ripple::verify( @@ -217,6 +228,14 @@ Manifest::revoked() const The maximum possible sequence number means that the master key has been revoked. */ + return revoked(sequence); +} + +bool +Manifest::revoked(std::uint32_t sequence) +{ + // The maximum possible sequence number means that the master key has + // been revoked. return sequence == std::numeric_limits::max(); } @@ -287,7 +306,7 @@ loadValidatorToken(std::vector const& blob, beast::Journal journal) } } -PublicKey +std::optional ManifestCache::getSigningKey(PublicKey const& pk) const { std::shared_lock lock{mutex_}; @@ -423,9 +442,18 @@ ManifestCache::applyManifest(Manifest m) if (!revoked) { + if (!m.signingKey) + { + JLOG(j_.warn()) << to_string(m) + << ": is not revoked and the manifest has no " + "signing key. Hence, the manifest is " + "invalid"; + return ManifestDisposition::invalid; + } + // Sanity check: the ephemeral key of this manifest should not be // used as the master or ephemeral key of another manifest: - if (auto const x = signingToMasterKeys_.find(m.signingKey); + if (auto const x = signingToMasterKeys_.find(*m.signingKey); x != signingToMasterKeys_.end()) { JLOG(j_.warn()) @@ -436,7 +464,7 @@ ManifestCache::applyManifest(Manifest m) return ManifestDisposition::badEphemeralKey; } - if (auto const x = map_.find(m.signingKey); x != map_.end()) + if (auto const x = map_.find(*m.signingKey); x != map_.end()) { JLOG(j_.warn()) << to_string(m) << ": Ephemeral key used as master key for " @@ -479,7 +507,7 @@ ManifestCache::applyManifest(Manifest m) logMftAct(stream, "AcceptedNew", m.masterKey, m.sequence); if (!revoked) - signingToMasterKeys_[m.signingKey] = m.masterKey; + signingToMasterKeys_.emplace(*m.signingKey, m.masterKey); auto masterKey = m.masterKey; map_.emplace(std::move(masterKey), std::move(m)); @@ -496,10 +524,10 @@ ManifestCache::applyManifest(Manifest m) m.sequence, iter->second.sequence); - signingToMasterKeys_.erase(iter->second.signingKey); + signingToMasterKeys_.erase(*iter->second.signingKey); if (!revoked) - signingToMasterKeys_[m.signingKey] = m.masterKey; + signingToMasterKeys_.emplace(*m.signingKey, m.masterKey); iter->second = std::move(m); diff --git a/src/ripple/app/misc/impl/ValidatorKeys.cpp b/src/ripple/app/misc/impl/ValidatorKeys.cpp index f9e55ae45a6..8da1992a2ef 100644 --- a/src/ripple/app/misc/impl/ValidatorKeys.cpp +++ b/src/ripple/app/misc/impl/ValidatorKeys.cpp @@ -56,9 +56,7 @@ ValidatorKeys::ValidatorKeys(Config const& config, beast::Journal j) } else { - secretKey = token->validationSecret; - publicKey = pk; - masterPublicKey = m->masterKey; + keys.emplace(m->masterKey, pk, token->validationSecret); nodeID = calcNodeID(m->masterKey); sequence = m->sequence; manifest = std::move(token->manifest); @@ -83,10 +81,10 @@ ValidatorKeys::ValidatorKeys(Config const& config, beast::Journal j) } else { - secretKey = generateSecretKey(KeyType::secp256k1, *seed); - publicKey = derivePublicKey(KeyType::secp256k1, secretKey); - masterPublicKey = publicKey; - nodeID = calcNodeID(publicKey); + SecretKey const sk = generateSecretKey(KeyType::secp256k1, *seed); + PublicKey const pk = derivePublicKey(KeyType::secp256k1, sk); + keys.emplace(pk, pk, sk); + nodeID = calcNodeID(pk); sequence = 0; } } diff --git a/src/ripple/app/misc/impl/ValidatorList.cpp b/src/ripple/app/misc/impl/ValidatorList.cpp index d17b85c4840..ff5fbd90eac 100644 --- a/src/ripple/app/misc/impl/ValidatorList.cpp +++ b/src/ripple/app/misc/impl/ValidatorList.cpp @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -134,7 +135,7 @@ ValidatorList::ValidatorList( bool ValidatorList::load( - PublicKey const& localSigningKey, + std::optional const& localSigningKey, std::vector const& configKeys, std::vector const& publisherKeys) { @@ -192,11 +193,12 @@ ValidatorList::load( JLOG(j_.debug()) << "Loaded " << count << " keys"; - localPubKey_ = validatorManifests_.getMasterKey(localSigningKey); + if (localSigningKey) + localPubKey_ = validatorManifests_.getMasterKey(*localSigningKey); // Treat local validator key as though it was listed in the config - if (localPubKey_.size()) - keyListings_.insert({localPubKey_, 1}); + if (localPubKey_) + keyListings_.insert({*localPubKey_, 1}); JLOG(j_.debug()) << "Loading configured validator keys"; @@ -231,16 +233,16 @@ ValidatorList::load( JLOG(j_.warn()) << "Duplicate node identity: " << match[1]; continue; } - auto [it, inserted] = publisherLists_.emplace(); - // Config listed keys never expire - auto& current = it->second.current; - if (inserted) - current.validUntil = TimeKeeper::time_point::max(); - current.list.emplace_back(*id); - it->second.status = PublisherStatus::available; + localPublisherList.list.emplace_back(*id); ++count; } + // Config listed keys never expire + // set the expiration time for the newly created publisher list + // exactly once + if (count > 0) + localPublisherList.validUntil = TimeKeeper::time_point::max(); + JLOG(j_.debug()) << "Loaded " << count << " entries"; return true; @@ -883,9 +885,11 @@ ValidatorList::applyListsAndBroadcast( if (disposition == ListDisposition::accepted) { bool good = true; - for (auto const& [pubKey, listCollection] : publisherLists_) + + // localPublisherList never expires, so localPublisherList is excluded + // from the below check. + for (auto const& [_, listCollection] : publisherLists_) { - (void)pubKey; if (listCollection.status != PublisherStatus::available) { good = false; @@ -899,12 +903,15 @@ ValidatorList::applyListsAndBroadcast( } bool broadcast = disposition <= ListDisposition::known_sequence; - if (broadcast) + // this function is only called for PublicKeys which are not specified + // in the config file (Note: Keys specified in the local config file are + // stored in ValidatorList::localPublisherList data member). + if (broadcast && result.status <= PublisherStatus::expired && + result.publisherKey && + publisherLists_[*result.publisherKey].maxSequence) { auto const& pubCollection = publisherLists_[*result.publisherKey]; - assert( - result.status <= PublisherStatus::expired && result.publisherKey && - pubCollection.maxSequence); + broadcastBlobs( *result.publisherKey, pubCollection, @@ -1070,9 +1077,24 @@ ValidatorList::applyList( using namespace std::string_literals; Json::Value list; - PublicKey pubKey; auto const& manifest = localManifest ? *localManifest : globalManifest; - auto const result = verify(lock, list, pubKey, manifest, blob, signature); + auto [result, pubKeyOpt] = verify(lock, list, manifest, blob, signature); + + if (!pubKeyOpt) + { + JLOG(j_.info()) << "ValidatorList::applyList unable to retrieve the " + "master public key from the verify function\n"; + return PublisherListStats{result}; + } + + if (!publicKeyType(*pubKeyOpt)) + { + JLOG(j_.info()) << "ValidatorList::applyList Invalid Public Key type" + " retrieved from the verify function\n "; + return PublisherListStats{result}; + } + + PublicKey pubKey = *pubKeyOpt; if (result > ListDisposition::pending) { if (publisherLists_.count(pubKey)) @@ -1255,11 +1277,12 @@ ValidatorList::loadLists() return sites; } -ListDisposition +// The returned PublicKey value is read from the manifest. Manifests do not +// contain the default-constructed public keys +std::pair> ValidatorList::verify( ValidatorList::lock_guard const& lock, Json::Value& list, - PublicKey& pubKey, std::string const& manifest, std::string const& blob, std::string const& signature) @@ -1267,35 +1290,33 @@ ValidatorList::verify( auto m = deserializeManifest(base64_decode(manifest)); if (!m || !publisherLists_.count(m->masterKey)) - return ListDisposition::untrusted; + return {ListDisposition::untrusted, {}}; - pubKey = m->masterKey; + PublicKey masterPubKey = m->masterKey; auto const revoked = m->revoked(); auto const result = publisherManifests_.applyManifest(std::move(*m)); if (revoked && result == ManifestDisposition::accepted) { - removePublisherList(lock, pubKey, PublisherStatus::revoked); + removePublisherList(lock, masterPubKey, PublisherStatus::revoked); // If the manifest is revoked, no future list is valid either - publisherLists_[pubKey].remaining.clear(); + publisherLists_[masterPubKey].remaining.clear(); } - if (revoked || result == ManifestDisposition::invalid) - return ListDisposition::untrusted; + auto const signingKey = publisherManifests_.getSigningKey(masterPubKey); + + if (revoked || !signingKey || result == ManifestDisposition::invalid) + return {ListDisposition::untrusted, masterPubKey}; auto const sig = strUnHex(signature); auto const data = base64_decode(blob); - if (!sig || - !ripple::verify( - publisherManifests_.getSigningKey(pubKey), - makeSlice(data), - makeSlice(*sig))) - return ListDisposition::invalid; + if (!sig || !ripple::verify(*signingKey, makeSlice(data), makeSlice(*sig))) + return {ListDisposition::invalid, masterPubKey}; Json::Reader r; if (!r.parse(data, list)) - return ListDisposition::invalid; + return {ListDisposition::invalid, masterPubKey}; if (list.isMember(jss::sequence) && list[jss::sequence].isInt() && list.isMember(jss::expiration) && list[jss::expiration].isInt() && @@ -1308,15 +1329,15 @@ ValidatorList::verify( auto const validUntil = TimeKeeper::time_point{ TimeKeeper::duration{list[jss::expiration].asUInt()}}; auto const now = timeKeeper_.now(); - auto const& listCollection = publisherLists_[pubKey]; + auto const& listCollection = publisherLists_[masterPubKey]; if (validUntil <= validFrom) - return ListDisposition::invalid; + return {ListDisposition::invalid, masterPubKey}; else if (sequence < listCollection.current.sequence) - return ListDisposition::stale; + return {ListDisposition::stale, masterPubKey}; else if (sequence == listCollection.current.sequence) - return ListDisposition::same_sequence; + return {ListDisposition::same_sequence, masterPubKey}; else if (validUntil <= now) - return ListDisposition::expired; + return {ListDisposition::expired, masterPubKey}; else if (validFrom > now) // Not yet valid. Return pending if one of the following is true // * There's no maxSequence, indicating this is the first blob seen @@ -1334,15 +1355,15 @@ ValidatorList::verify( validFrom < listCollection.remaining .at(*listCollection.maxSequence) .validFrom) - ? ListDisposition::pending - : ListDisposition::known_sequence; + ? std::make_pair(ListDisposition::pending, masterPubKey) + : std::make_pair(ListDisposition::known_sequence, masterPubKey); } else { - return ListDisposition::invalid; + return {ListDisposition::invalid, masterPubKey}; } - return ListDisposition::accepted; + return {ListDisposition::accepted, masterPubKey}; } bool @@ -1408,7 +1429,7 @@ ValidatorList::trustedPublisher(PublicKey const& identity) const publisherLists_.at(identity).status < PublisherStatus::revoked; } -PublicKey +std::optional ValidatorList::localPublicKey() const { std::shared_lock read_lock{mutex_}; @@ -1452,7 +1473,7 @@ ValidatorList::removePublisherList( std::size_t ValidatorList::count(ValidatorList::shared_lock const&) const { - return publisherLists_.size(); + return publisherLists_.size() + (localPublisherList.list.size() > 0); } std::size_t @@ -1466,13 +1487,14 @@ std::optional ValidatorList::expires(ValidatorList::shared_lock const&) const { std::optional res{}; - for (auto const& [pubKey, collection] : publisherLists_) + for (auto const& [_, collection] : publisherLists_) { - (void)pubKey; // Unfetched auto const& current = collection.current; if (current.validUntil == TimeKeeper::time_point{}) + { return std::nullopt; + } // Find the latest validUntil in a chain where the next validFrom // overlaps with the previous validUntil. applyLists has already cleaned @@ -1493,6 +1515,20 @@ ValidatorList::expires(ValidatorList::shared_lock const&) const res = chainedExpiration; } } + + if (localPublisherList.list.size() > 0) + { + PublisherList collection = localPublisherList; + // Unfetched + auto const& current = collection; + auto chainedExpiration = current.validUntil; + + // Earliest + if (!res || chainedExpiration < *res) + { + res = chainedExpiration; + } + } return res; } @@ -1541,23 +1577,18 @@ ValidatorList::getJson() const } } - // Local static keys - PublicKey local; + // Validator keys listed in the local config file Json::Value& jLocalStaticKeys = (res[jss::local_static_keys] = Json::arrayValue); - if (auto it = publisherLists_.find(local); it != publisherLists_.end()) - { - for (auto const& key : it->second.current.list) - jLocalStaticKeys.append(toBase58(TokenType::NodePublic, key)); - } + + for (auto const& key : localPublisherList.list) + jLocalStaticKeys.append(toBase58(TokenType::NodePublic, key)); // Publisher lists Json::Value& jPublisherLists = (res[jss::publisher_lists] = Json::arrayValue); for (auto const& [publicKey, pubCollection] : publisherLists_) { - if (local == publicKey) - continue; Json::Value& curr = jPublisherLists.append(Json::objectValue); curr[jss::pubkey_publisher] = strHex(publicKey); curr[jss::available] = @@ -1617,10 +1648,10 @@ ValidatorList::getJson() const validatorManifests_.for_each_manifest([&jSigningKeys, this](Manifest const& manifest) { auto it = keyListings_.find(manifest.masterKey); - if (it != keyListings_.end()) + if (it != keyListings_.end() && manifest.signingKey) { jSigningKeys[toBase58(TokenType::NodePublic, manifest.masterKey)] = - toBase58(TokenType::NodePublic, manifest.signingKey); + toBase58(TokenType::NodePublic, *manifest.signingKey); } }); @@ -1661,7 +1692,7 @@ ValidatorList::for_each_available( for (auto const& [key, plCollection] : publisherLists_) { - if (plCollection.status != PublisherStatus::available || key.empty()) + if (plCollection.status != PublisherStatus::available) continue; assert(plCollection.maxSequence); func( @@ -1781,6 +1812,9 @@ ValidatorList::updateTrusted( // Rotate pending and remove expired published lists bool good = true; + // localPublisherList is not processed here. This is because the + // Validators specified in the local config file do not expire nor do + // they have a "remaining" section of PublisherList. for (auto& [pubKey, collection] : publisherLists_) { { @@ -1836,6 +1870,8 @@ ValidatorList::updateTrusted( } } // Remove if expired + // ValidatorLists specified in the local config file never expire. + // Hence, the below steps are not relevant for localPublisherList if (collection.status == PublisherStatus::available && collection.current.validUntil <= closeTime) { @@ -1877,8 +1913,15 @@ ValidatorList::updateTrusted( { trustedSigningKeys_.clear(); + // trustedMasterKeys_ contain non-revoked manifests only. Hence the + // manifests must contain a valid signingKey for (auto const& k : trustedMasterKeys_) - trustedSigningKeys_.insert(validatorManifests_.getSigningKey(k)); + { + std::optional const signingKey = + validatorManifests_.getSigningKey(k); + assert(signingKey); + trustedSigningKeys_.insert(*signingKey); + } } JLOG(j_.debug()) @@ -1920,7 +1963,8 @@ ValidatorList::updateTrusted( << unlSize << ")"; } - if (publisherLists_.size() && unlSize == 0) + if ((publisherLists_.size() || localPublisherList.list.size()) && + unlSize == 0) { // No validators. Lock down. ops.setUNLBlocked(); diff --git a/src/ripple/basics/impl/contract.cpp b/src/ripple/basics/impl/contract.cpp index bf7df682587..3a50db38010 100644 --- a/src/ripple/basics/impl/contract.cpp +++ b/src/ripple/basics/impl/contract.cpp @@ -20,7 +20,6 @@ #include #include #include -#include #include namespace ripple { diff --git a/src/ripple/json/impl/json_reader.cpp b/src/ripple/json/impl/json_reader.cpp index 299accd84a8..001049bcdbc 100644 --- a/src/ripple/json/impl/json_reader.cpp +++ b/src/ripple/json/impl/json_reader.cpp @@ -22,7 +22,6 @@ #include #include -#include #include #include diff --git a/src/ripple/overlay/impl/OverlayImpl.cpp b/src/ripple/overlay/impl/OverlayImpl.cpp index 6ed046f0403..6b047b7ef81 100644 --- a/src/ripple/overlay/impl/OverlayImpl.cpp +++ b/src/ripple/overlay/impl/OverlayImpl.cpp @@ -473,7 +473,7 @@ OverlayImpl::start() PeerFinder::Config config = PeerFinder::Config::makeConfig( app_.config(), serverHandler_.setup().overlay.port, - !app_.getValidationPublicKey().empty(), + app_.getValidationPublicKey().has_value(), setup_.ipLimit); m_peerFinder->setConfig(config); diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index 0d58a10abac..f93c9f135ad 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -1574,7 +1574,9 @@ PeerImp::handleTransaction( flags |= SF_TRUSTED; } - if (app_.getValidationPublicKey().empty()) + // for non-validator nodes only -- localPublicKey is set for + // validators only + if (!app_.getValidationPublicKey()) { // For now, be paranoid and have each validator // check each transaction, regardless of source diff --git a/src/ripple/protocol/PublicKey.h b/src/ripple/protocol/PublicKey.h index 58394cd82d4..9cf1a456953 100644 --- a/src/ripple/protocol/PublicKey.h +++ b/src/ripple/protocol/PublicKey.h @@ -61,13 +61,17 @@ namespace ripple { class PublicKey { protected: - std::size_t size_ = 0; - std::uint8_t buf_[33]; // should be large enough + // All the constructed public keys are valid, non-empty and contain 33 + // bytes of data. + static constexpr std::size_t size_ = 33; + std::uint8_t buf_[size_]; // should be large enough public: using const_iterator = std::uint8_t const*; - PublicKey() = default; +public: + PublicKey() = delete; + PublicKey(PublicKey const& other); PublicKey& operator=(PublicKey const& other); @@ -115,12 +119,6 @@ class PublicKey return buf_ + size_; } - bool - empty() const noexcept - { - return size_ == 0; - } - Slice slice() const noexcept { @@ -141,8 +139,7 @@ operator<<(std::ostream& os, PublicKey const& pk); inline bool operator==(PublicKey const& lhs, PublicKey const& rhs) { - return lhs.size() == rhs.size() && - std::memcmp(lhs.data(), rhs.data(), rhs.size()) == 0; + return std::memcmp(lhs.data(), rhs.data(), rhs.size()) == 0; } inline bool diff --git a/src/ripple/protocol/SecretKey.h b/src/ripple/protocol/SecretKey.h index 3026fb9d775..824ae9b1e0f 100644 --- a/src/ripple/protocol/SecretKey.h +++ b/src/ripple/protocol/SecretKey.h @@ -41,7 +41,7 @@ class SecretKey public: using const_iterator = std::uint8_t const*; - SecretKey() = default; + SecretKey() = delete; SecretKey(SecretKey const&) = default; SecretKey& operator=(SecretKey const&) = default; diff --git a/src/ripple/protocol/impl/PublicKey.cpp b/src/ripple/protocol/impl/PublicKey.cpp index 8ab1bd46cdf..22cb351e61c 100644 --- a/src/ripple/protocol/impl/PublicKey.cpp +++ b/src/ripple/protocol/impl/PublicKey.cpp @@ -24,7 +24,6 @@ #include #include #include -#include namespace ripple { @@ -176,16 +175,19 @@ ed25519Canonical(Slice const& sig) PublicKey::PublicKey(Slice const& slice) { + if (slice.size() < size_) + LogicError( + "PublicKey::PublicKey - Input slice cannot be an undersized " + "buffer"); + if (!publicKeyType(slice)) LogicError("PublicKey::PublicKey invalid type"); - size_ = slice.size(); std::memcpy(buf_, slice.data(), size_); } -PublicKey::PublicKey(PublicKey const& other) : size_(other.size_) +PublicKey::PublicKey(PublicKey const& other) { - if (size_) - std::memcpy(buf_, other.buf_, size_); + std::memcpy(buf_, other.buf_, size_); } PublicKey& @@ -193,9 +195,7 @@ PublicKey::operator=(PublicKey const& other) { if (this != &other) { - size_ = other.size_; - if (size_) - std::memcpy(buf_, other.buf_, size_); + std::memcpy(buf_, other.buf_, size_); } return *this; diff --git a/src/ripple/rpc/handlers/Manifest.cpp b/src/ripple/rpc/handlers/Manifest.cpp index 50a9edf88f6..daf6cc22661 100644 --- a/src/ripple/rpc/handlers/Manifest.cpp +++ b/src/ripple/rpc/handlers/Manifest.cpp @@ -51,14 +51,13 @@ doManifest(RPC::JsonContext& context) // first attempt to use params as ephemeral key, // if this lookup succeeds master key will be returned, - // else pk will just be returned and we will assume that - // is master key anyways + // else an unseated optional is returned auto const mk = context.app.validatorManifests().getMasterKey(*pk); auto const ek = context.app.validatorManifests().getSigningKey(mk); // if ephemeral key not found, we don't have specified manifest - if (ek == mk) + if (!ek) return ret; if (auto const manifest = context.app.validatorManifests().getManifest(mk)) @@ -66,7 +65,7 @@ doManifest(RPC::JsonContext& context) Json::Value details; details[jss::master_key] = toBase58(TokenType::NodePublic, mk); - details[jss::ephemeral_key] = toBase58(TokenType::NodePublic, ek); + details[jss::ephemeral_key] = toBase58(TokenType::NodePublic, *ek); if (auto const seq = context.app.validatorManifests().getSequence(mk)) details[jss::seq] = *seq; diff --git a/src/ripple/rpc/handlers/PayChanClaim.cpp b/src/ripple/rpc/handlers/PayChanClaim.cpp index 23a4041bb35..ca3e8c63c6d 100644 --- a/src/ripple/rpc/handlers/PayChanClaim.cpp +++ b/src/ripple/rpc/handlers/PayChanClaim.cpp @@ -55,11 +55,16 @@ doChannelAuthorize(RPC::JsonContext& context) return RPC::missing_field_error(jss::secret); Json::Value result; - auto const [pk, sk] = + std::optional> const keyPair = RPC::keypairForSignature(params, result, context.apiVersion); - if (RPC::contains_error(result)) + + assert(keyPair || RPC::contains_error(result)); + if (!keyPair || RPC::contains_error(result)) return result; + PublicKey const& pk = keyPair->first; + SecretKey const& sk = keyPair->second; + uint256 channelId; if (!channelId.parseHex(params[jss::channel_id].asString())) return rpcError(rpcCHANNEL_MALFORMED); diff --git a/src/ripple/rpc/handlers/ValidatorInfo.cpp b/src/ripple/rpc/handlers/ValidatorInfo.cpp index 93859dd65d5..910c5e9740f 100644 --- a/src/ripple/rpc/handlers/ValidatorInfo.cpp +++ b/src/ripple/rpc/handlers/ValidatorInfo.cpp @@ -30,24 +30,23 @@ Json::Value doValidatorInfo(RPC::JsonContext& context) { // return error if not configured as validator - if (context.app.getValidationPublicKey().empty()) + auto const validationPK = context.app.getValidationPublicKey(); + if (!validationPK) return RPC::not_validator_error(); Json::Value ret; - auto const pk = context.app.getValidationPublicKey(); - - // assume pk is ephemeral key, get master key - auto const mk = context.app.validatorManifests().getMasterKey(pk); + // assume validationPK is ephemeral key, get master key + auto const mk = + context.app.validatorManifests().getMasterKey(*validationPK); ret[jss::master_key] = toBase58(TokenType::NodePublic, mk); - // pk is maskter key, eg no ephemeral key, eg no manifest, just return - if (mk == pk) + // validationPK is master key, this implies that there is no ephemeral + // key, no manifest, hence return + if (mk == validationPK) return ret; - // lookup ephemeral key - auto const ek = context.app.validatorManifests().getSigningKey(mk); - ret[jss::ephemeral_key] = toBase58(TokenType::NodePublic, ek); + ret[jss::ephemeral_key] = toBase58(TokenType::NodePublic, *validationPK); if (auto const manifest = context.app.validatorManifests().getManifest(mk)) ret[jss::manifest] = base64_encode(*manifest); diff --git a/src/ripple/rpc/impl/RPCHelpers.cpp b/src/ripple/rpc/impl/RPCHelpers.cpp index d7fbaccbc40..449586d7b99 100644 --- a/src/ripple/rpc/impl/RPCHelpers.cpp +++ b/src/ripple/rpc/impl/RPCHelpers.cpp @@ -792,7 +792,7 @@ getSeedFromRPC(Json::Value const& params, Json::Value& error) return seed; } -std::pair +std::optional> keypairForSignature( Json::Value const& params, Json::Value& error, diff --git a/src/ripple/rpc/impl/RPCHelpers.h b/src/ripple/rpc/impl/RPCHelpers.h index c28c2d0f244..8b6460cd451 100644 --- a/src/ripple/rpc/impl/RPCHelpers.h +++ b/src/ripple/rpc/impl/RPCHelpers.h @@ -287,7 +287,7 @@ getAPIVersionNumber(const Json::Value& value, bool betaEnabled); std::variant, Json::Value> getLedgerByContext(RPC::JsonContext& context); -std::pair +std::optional> keypairForSignature( Json::Value const& params, Json::Value& error, diff --git a/src/ripple/rpc/impl/TransactionSign.cpp b/src/ripple/rpc/impl/TransactionSign.cpp index 0881881bd1a..a80ae4b150e 100644 --- a/src/ripple/rpc/impl/TransactionSign.cpp +++ b/src/ripple/rpc/impl/TransactionSign.cpp @@ -53,35 +53,25 @@ class SigningForParams { private: AccountID const* const multiSigningAcctID_; - PublicKey* const multiSignPublicKey_; - Buffer* const multiSignature_; + std::optional multiSignPublicKey_; + Buffer multiSignature_; public: - explicit SigningForParams() - : multiSigningAcctID_(nullptr) - , multiSignPublicKey_(nullptr) - , multiSignature_(nullptr) + explicit SigningForParams() : multiSigningAcctID_(nullptr) { } SigningForParams(SigningForParams const& rhs) = delete; - SigningForParams( - AccountID const& multiSigningAcctID, - PublicKey& multiSignPublicKey, - Buffer& multiSignature) + SigningForParams(AccountID const& multiSigningAcctID) : multiSigningAcctID_(&multiSigningAcctID) - , multiSignPublicKey_(&multiSignPublicKey) - , multiSignature_(&multiSignature) { } bool isMultiSigning() const { - return ( - (multiSigningAcctID_ != nullptr) && - (multiSignPublicKey_ != nullptr) && (multiSignature_ != nullptr)); + return multiSigningAcctID_ != nullptr; } bool @@ -97,23 +87,46 @@ class SigningForParams return !isMultiSigning(); } + bool + validMultiSign() const + { + return isMultiSigning() && multiSignPublicKey_ && + multiSignature_.size(); + } + // Don't call this method unless isMultiSigning() returns true. AccountID const& - getSigner() + getSigner() const { + if (!multiSigningAcctID_) + LogicError("Accessing unknown SigningForParams::getSigner()"); return *multiSigningAcctID_; } + PublicKey const& + getPublicKey() const + { + if (!multiSignPublicKey_) + LogicError("Accessing unknown SigningForParams::getPublicKey()"); + return *multiSignPublicKey_; + } + + Buffer const& + getSignature() const + { + return multiSignature_; + } + void setPublicKey(PublicKey const& multiSignPublicKey) { - *multiSignPublicKey_ = multiSignPublicKey; + multiSignPublicKey_ = multiSignPublicKey; } void moveMultiSignature(Buffer&& multiSignature) { - *multiSignature_ = std::move(multiSignature); + multiSignature_ = std::move(multiSignature); } }; @@ -386,10 +399,14 @@ transactionPreProcessImpl( auto j = app.journal("RPCHandler"); Json::Value jvResult; - auto const [pk, sk] = keypairForSignature(params, jvResult); - if (contains_error(jvResult)) + std::optional> keyPair = + keypairForSignature(params, jvResult); + if (!keyPair || contains_error(jvResult)) return jvResult; + PublicKey const& pk = keyPair->first; + SecretKey const& sk = keyPair->second; + bool const verify = !(params.isMember(jss::offline) && params[jss::offline].asBool()); @@ -1009,10 +1026,7 @@ transactionSignFor( } // Add and amend fields based on the transaction type. - Buffer multiSignature; - PublicKey multiSignPubKey; - SigningForParams signForParams( - *signerAccountID, multiSignPubKey, multiSignature); + SigningForParams signForParams(*signerAccountID); transactionPreProcessResult preprocResult = transactionPreProcessImpl( jvRequest, role, signForParams, validatedLedgerAge, app); @@ -1020,12 +1034,14 @@ transactionSignFor( if (!preprocResult.second) return preprocResult.first; + assert(signForParams.validMultiSign()); + { std::shared_ptr account_state = ledger->read(keylet::account(*signerAccountID)); // Make sure the account and secret belong together. - auto const err = - acctMatchesPubKey(account_state, *signerAccountID, multiSignPubKey); + auto const err = acctMatchesPubKey( + account_state, *signerAccountID, signForParams.getPublicKey()); if (err != rpcSUCCESS) return rpcError(err); @@ -1037,8 +1053,9 @@ transactionSignFor( // Make the signer object that we'll inject. STObject signer(sfSigner); signer[sfAccount] = *signerAccountID; - signer.setFieldVL(sfTxnSignature, multiSignature); - signer.setFieldVL(sfSigningPubKey, multiSignPubKey.slice()); + signer.setFieldVL(sfTxnSignature, signForParams.getSignature()); + signer.setFieldVL( + sfSigningPubKey, signForParams.getPublicKey().slice()); // If there is not yet a Signers array, make one. if (!sttx->isFieldPresent(sfSigners)) diff --git a/src/test/app/LedgerLoad_test.cpp b/src/test/app/LedgerLoad_test.cpp index 93fc002ae2a..f06e7d0bf01 100644 --- a/src/test/app/LedgerLoad_test.cpp +++ b/src/test/app/LedgerLoad_test.cpp @@ -63,21 +63,21 @@ class LedgerLoad_test : public beast::unit_test::suite retval.ledgerFile = td.file("ledgerdata.json"); Env env{*this}; - Account prev; + std::optional prev; for (auto i = 0; i < 20; ++i) { Account acct{"A" + std::to_string(i)}; env.fund(XRP(10000), acct); env.close(); - if (i > 0) + if (i > 0 && BEAST_EXPECT(prev)) { - env.trust(acct["USD"](1000), prev); - env(pay(acct, prev, acct["USD"](5))); + env.trust(acct["USD"](1000), *prev); + env(pay(acct, *prev, acct["USD"](5))); } env(offer(acct, XRP(100), acct["USD"](1))); env.close(); - prev = std::move(acct); + prev.emplace(std::move(acct)); } retval.ledger = env.rpc("ledger", "current", "full")[jss::result]; diff --git a/src/test/app/LedgerReplay_test.cpp b/src/test/app/LedgerReplay_test.cpp index 41c9a71e9f6..aee24cd7d57 100644 --- a/src/test/app/LedgerReplay_test.cpp +++ b/src/test/app/LedgerReplay_test.cpp @@ -197,7 +197,9 @@ enum class PeerFeature { class TestPeer : public Peer { public: - TestPeer(bool enableLedgerReplay) : ledgerReplayEnabled_(enableLedgerReplay) + TestPeer(bool enableLedgerReplay) + : ledgerReplayEnabled_(enableLedgerReplay) + , nodePublicKey_(derivePublicKey(KeyType::ed25519, randomSecretKey())) { } @@ -237,8 +239,7 @@ class TestPeer : public Peer PublicKey const& getNodePublic() const override { - static PublicKey key{}; - return key; + return nodePublicKey_; } Json::Value json() override @@ -314,6 +315,7 @@ class TestPeer : public Peer } bool ledgerReplayEnabled_; + PublicKey nodePublicKey_; }; enum class PeerSetBehavior { diff --git a/src/test/app/Manifest_test.cpp b/src/test/app/Manifest_test.cpp index db66e09e518..b72623309e9 100644 --- a/src/test/app/Manifest_test.cpp +++ b/src/test/app/Manifest_test.cpp @@ -238,12 +238,8 @@ class Manifest_test : public beast::unit_test::suite Manifest clone(Manifest const& m) { - Manifest m2; - m2.serialized = m.serialized; - m2.masterKey = m.masterKey; - m2.signingKey = m.signingKey; - m2.sequence = m.sequence; - m2.domain = m.domain; + Manifest m2( + m.serialized, m.masterKey, m.signingKey, m.sequence, m.domain); return m2; } @@ -313,14 +309,13 @@ class Manifest_test : public beast::unit_test::suite } { // save should store all trusted master keys to db - PublicKey emptyLocalKey; std::vector s1; std::vector keys; std::string cfgManifest; for (auto const& man : inManifests) s1.push_back( toBase58(TokenType::NodePublic, man->masterKey)); - unl->load(emptyLocalKey, s1, keys); + unl->load({}, s1, keys); m.save( *dbCon, @@ -852,7 +847,10 @@ class Manifest_test : public beast::unit_test::suite BEAST_EXPECT(manifest); BEAST_EXPECT(manifest->masterKey == pk); - BEAST_EXPECT(manifest->signingKey == PublicKey()); + + // Since this manifest is revoked, it should not have + // a signingKey + BEAST_EXPECT(!manifest->signingKey); BEAST_EXPECT(manifest->revoked()); BEAST_EXPECT(manifest->domain.empty()); BEAST_EXPECT(manifest->serialized == m); diff --git a/src/test/app/ValidatorKeys_test.cpp b/src/test/app/ValidatorKeys_test.cpp index 3943fd85881..99bd6188261 100644 --- a/src/test/app/ValidatorKeys_test.cpp +++ b/src/test/app/ValidatorKeys_test.cpp @@ -107,7 +107,7 @@ class ValidatorKeys_test : public beast::unit_test::suite // No config -> no key but valid Config c; ValidatorKeys k{c, journal}; - BEAST_EXPECT(k.publicKey.size() == 0); + BEAST_EXPECT(!k.keys); BEAST_EXPECT(k.manifest.empty()); BEAST_EXPECT(!k.configInvalid()); } @@ -117,8 +117,11 @@ class ValidatorKeys_test : public beast::unit_test::suite c.section(SECTION_VALIDATION_SEED).append(seed); ValidatorKeys k{c, journal}; - BEAST_EXPECT(k.publicKey == seedPublicKey); - BEAST_EXPECT(k.secretKey == seedSecretKey); + if (BEAST_EXPECT(k.keys)) + { + BEAST_EXPECT(k.keys->publicKey == seedPublicKey); + BEAST_EXPECT(k.keys->secretKey == seedSecretKey); + } BEAST_EXPECT(k.nodeID == seedNodeID); BEAST_EXPECT(k.manifest.empty()); BEAST_EXPECT(!k.configInvalid()); @@ -131,7 +134,7 @@ class ValidatorKeys_test : public beast::unit_test::suite ValidatorKeys k{c, journal}; BEAST_EXPECT(k.configInvalid()); - BEAST_EXPECT(k.publicKey.size() == 0); + BEAST_EXPECT(!k.keys); BEAST_EXPECT(k.manifest.empty()); } @@ -141,8 +144,11 @@ class ValidatorKeys_test : public beast::unit_test::suite c.section(SECTION_VALIDATOR_TOKEN).append(tokenBlob); ValidatorKeys k{c, journal}; - BEAST_EXPECT(k.publicKey == tokenPublicKey); - BEAST_EXPECT(k.secretKey == tokenSecretKey); + if (BEAST_EXPECT(k.keys)) + { + BEAST_EXPECT(k.keys->publicKey == tokenPublicKey); + BEAST_EXPECT(k.keys->secretKey == tokenSecretKey); + } BEAST_EXPECT(k.nodeID == tokenNodeID); BEAST_EXPECT(k.manifest == tokenManifest); BEAST_EXPECT(!k.configInvalid()); @@ -153,7 +159,7 @@ class ValidatorKeys_test : public beast::unit_test::suite c.section(SECTION_VALIDATOR_TOKEN).append("badtoken"); ValidatorKeys k{c, journal}; BEAST_EXPECT(k.configInvalid()); - BEAST_EXPECT(k.publicKey.size() == 0); + BEAST_EXPECT(!k.keys); BEAST_EXPECT(k.manifest.empty()); } @@ -165,7 +171,7 @@ class ValidatorKeys_test : public beast::unit_test::suite ValidatorKeys k{c, journal}; BEAST_EXPECT(k.configInvalid()); - BEAST_EXPECT(k.publicKey.size() == 0); + BEAST_EXPECT(!k.keys); BEAST_EXPECT(k.manifest.empty()); } @@ -176,7 +182,7 @@ class ValidatorKeys_test : public beast::unit_test::suite ValidatorKeys k{c, journal}; BEAST_EXPECT(k.configInvalid()); - BEAST_EXPECT(k.publicKey.size() == 0); + BEAST_EXPECT(!k.keys); BEAST_EXPECT(k.manifest.empty()); } } diff --git a/src/test/app/ValidatorList_test.cpp b/src/test/app/ValidatorList_test.cpp index ff9b57f3ced..638c2060d32 100644 --- a/src/test/app/ValidatorList_test.cpp +++ b/src/test/app/ValidatorList_test.cpp @@ -221,7 +221,6 @@ class ValidatorList_test : public beast::unit_test::suite jtx::Env env( *this, jtx::envconfig(), nullptr, beast::severities::kDisabled); auto& app = env.app(); - PublicKey emptyLocalKey; std::vector const emptyCfgKeys; std::vector const emptyCfgPublishers; @@ -278,8 +277,8 @@ class ValidatorList_test : public beast::unit_test::suite env.journal); // Correct (empty) configuration - BEAST_EXPECT(trustedKeys->load( - emptyLocalKey, emptyCfgKeys, emptyCfgPublishers)); + BEAST_EXPECT( + trustedKeys->load({}, emptyCfgKeys, emptyCfgPublishers)); // load local validator key with or without manifest BEAST_EXPECT(trustedKeys->load( @@ -303,8 +302,7 @@ class ValidatorList_test : public beast::unit_test::suite app.config().legacy("database_path"), env.journal); - BEAST_EXPECT( - trustedKeys->load(emptyLocalKey, cfgKeys, emptyCfgPublishers)); + BEAST_EXPECT(trustedKeys->load({}, cfgKeys, emptyCfgPublishers)); for (auto const& n : configList) BEAST_EXPECT(trustedKeys->listed(n)); @@ -315,23 +313,21 @@ class ValidatorList_test : public beast::unit_test::suite std::vector cfgMasterKeys( {format(masterNode1), format(masterNode2, " Comment")}); - BEAST_EXPECT(trustedKeys->load( - emptyLocalKey, cfgMasterKeys, emptyCfgPublishers)); + BEAST_EXPECT( + trustedKeys->load({}, cfgMasterKeys, emptyCfgPublishers)); BEAST_EXPECT(trustedKeys->listed(masterNode1)); BEAST_EXPECT(trustedKeys->listed(masterNode2)); // load should reject invalid config keys + BEAST_EXPECT( + !trustedKeys->load({}, {"NotAPublicKey"}, emptyCfgPublishers)); BEAST_EXPECT(!trustedKeys->load( - emptyLocalKey, {"NotAPublicKey"}, emptyCfgPublishers)); - BEAST_EXPECT(!trustedKeys->load( - emptyLocalKey, - {format(randomNode(), "!")}, - emptyCfgPublishers)); + {}, {format(randomNode(), "!")}, emptyCfgPublishers)); // load terminates when encountering an invalid entry auto const goodKey = randomNode(); BEAST_EXPECT(!trustedKeys->load( - emptyLocalKey, + {}, {format(randomNode(), "!"), format(goodKey)}, emptyCfgPublishers)); BEAST_EXPECT(!trustedKeys->listed(goodKey)); @@ -408,8 +404,7 @@ class ValidatorList_test : public beast::unit_test::suite // load should reject invalid validator list signing keys std::vector badPublishers({"NotASigningKey"}); - BEAST_EXPECT( - !trustedKeys->load(emptyLocalKey, emptyCfgKeys, badPublishers)); + BEAST_EXPECT(!trustedKeys->load({}, emptyCfgKeys, badPublishers)); // load should reject validator list signing keys with invalid // encoding @@ -419,8 +414,7 @@ class ValidatorList_test : public beast::unit_test::suite for (auto const& key : keys) badPublishers.push_back(toBase58(TokenType::NodePublic, key)); - BEAST_EXPECT( - !trustedKeys->load(emptyLocalKey, emptyCfgKeys, badPublishers)); + BEAST_EXPECT(!trustedKeys->load({}, emptyCfgKeys, badPublishers)); for (auto const& key : keys) BEAST_EXPECT(!trustedKeys->trustedPublisher(key)); @@ -429,8 +423,7 @@ class ValidatorList_test : public beast::unit_test::suite for (auto const& key : keys) cfgPublishers.push_back(strHex(key)); - BEAST_EXPECT( - trustedKeys->load(emptyLocalKey, emptyCfgKeys, cfgPublishers)); + BEAST_EXPECT(trustedKeys->load({}, emptyCfgKeys, cfgPublishers)); for (auto const& key : keys) BEAST_EXPECT(trustedKeys->trustedPublisher(key)); } @@ -464,8 +457,7 @@ class ValidatorList_test : public beast::unit_test::suite std::vector cfgPublishers = { strHex(pubRevokedPublic), strHex(legitKey)}; - BEAST_EXPECT( - trustedKeys->load(emptyLocalKey, emptyCfgKeys, cfgPublishers)); + BEAST_EXPECT(trustedKeys->load({}, emptyCfgKeys, cfgPublishers)); BEAST_EXPECT(!trustedKeys->trustedPublisher(pubRevokedPublic)); BEAST_EXPECT(trustedKeys->trustedPublisher(legitKey)); @@ -569,10 +561,9 @@ class ValidatorList_test : public beast::unit_test::suite 1)); std::vector cfgKeys1({strHex(publisherPublic)}); - PublicKey emptyLocalKey; std::vector emptyCfgKeys; - BEAST_EXPECT(trustedKeys->load(emptyLocalKey, emptyCfgKeys, cfgKeys1)); + BEAST_EXPECT(trustedKeys->load({}, emptyCfgKeys, cfgKeys1)); std::map> const lists = []() { auto constexpr listSize = 20; @@ -954,10 +945,9 @@ class ValidatorList_test : public beast::unit_test::suite 1)); std::vector cfgKeys1({strHex(publisherPublic)}); - PublicKey emptyLocalKey; std::vector emptyCfgKeys; - BEAST_EXPECT(trustedKeys->load(emptyLocalKey, emptyCfgKeys, cfgKeys1)); + BEAST_EXPECT(trustedKeys->load({}, emptyCfgKeys, cfgKeys1)); std::vector const list = []() { auto constexpr listSize = 20; @@ -1066,7 +1056,6 @@ class ValidatorList_test : public beast::unit_test::suite std::string const siteUri = "testUpdateTrusted.test"; - PublicKey emptyLocalKeyOuter; ManifestCache manifestsOuter; jtx::Env env(*this); auto& app = env.app(); @@ -1096,8 +1085,8 @@ class ValidatorList_test : public beast::unit_test::suite unseenValidators.emplace(calcNodeID(valKey)); } - BEAST_EXPECT(trustedKeysOuter->load( - emptyLocalKeyOuter, cfgKeys, cfgPublishersOuter)); + BEAST_EXPECT( + trustedKeysOuter->load({}, cfgKeys, cfgPublishersOuter)); // updateTrusted should make all configured validators trusted // even if they are not active/seen @@ -1147,8 +1136,8 @@ class ValidatorList_test : public beast::unit_test::suite std::vector cfgKeys( {toBase58(TokenType::NodePublic, masterPublic)}); - BEAST_EXPECT(trustedKeysOuter->load( - emptyLocalKeyOuter, cfgKeys, cfgPublishersOuter)); + BEAST_EXPECT( + trustedKeysOuter->load({}, cfgKeys, cfgPublishersOuter)); auto const signingKeys1 = randomKeyPair(KeyType::secp256k1); auto const signingPublic1 = signingKeys1.first; @@ -1260,8 +1249,7 @@ class ValidatorList_test : public beast::unit_test::suite std::vector cfgPublishers({strHex(publisherPublic)}); std::vector emptyCfgKeys; - BEAST_EXPECT(trustedKeys->load( - emptyLocalKeyOuter, emptyCfgKeys, cfgPublishers)); + BEAST_EXPECT(trustedKeys->load({}, emptyCfgKeys, cfgPublishers)); TrustChanges changes = trustedKeys->updateTrusted( activeValidatorsOuter, @@ -1305,8 +1293,7 @@ class ValidatorList_test : public beast::unit_test::suite toBeSeen = calcNodeID(valKey); } - BEAST_EXPECT(trustedKeys->load( - emptyLocalKeyOuter, cfgKeys, cfgPublishersOuter)); + BEAST_EXPECT(trustedKeys->load({}, cfgKeys, cfgPublishersOuter)); TrustChanges changes = trustedKeys->updateTrusted( activeValidators, @@ -1339,7 +1326,6 @@ class ValidatorList_test : public beast::unit_test::suite app.config().legacy("database_path"), env.journal); - PublicKey emptyLocalKey; std::vector emptyCfgKeys; auto const publisherKeys = randomKeyPair(KeyType::secp256k1); auto const pubSigningKeys = randomKeyPair(KeyType::secp256k1); @@ -1352,8 +1338,7 @@ class ValidatorList_test : public beast::unit_test::suite std::vector cfgKeys({strHex(publisherKeys.first)}); - BEAST_EXPECT( - trustedKeys->load(emptyLocalKey, emptyCfgKeys, cfgKeys)); + BEAST_EXPECT(trustedKeys->load({}, emptyCfgKeys, cfgKeys)); std::vector list({randomValidator(), randomValidator()}); hash_set activeValidators( @@ -1463,8 +1448,7 @@ class ValidatorList_test : public beast::unit_test::suite cfgKeys.push_back(toBase58(TokenType::NodePublic, valKey)); activeValidators.emplace(calcNodeID(valKey)); activeKeys.emplace(valKey); - BEAST_EXPECT(trustedKeys->load( - emptyLocalKeyOuter, cfgKeys, cfgPublishers)); + BEAST_EXPECT(trustedKeys->load({}, cfgKeys, cfgPublishers)); TrustChanges changes = trustedKeys->updateTrusted( activeValidators, env.timeKeeper().now(), @@ -1564,11 +1548,10 @@ class ValidatorList_test : public beast::unit_test::suite std::vector cfgPublishers( {strHex(publisherPublic)}); - PublicKey emptyLocalKey; std::vector emptyCfgKeys; - BEAST_EXPECT(trustedKeys->load( - emptyLocalKey, emptyCfgKeys, cfgPublishers)); + BEAST_EXPECT( + trustedKeys->load({}, emptyCfgKeys, cfgPublishers)); auto const version = 1; auto const sequence = 1; @@ -1640,9 +1623,8 @@ class ValidatorList_test : public beast::unit_test::suite BEAST_EXPECT(trustedKeys->expires() == std::nullopt); // Config listed keys have maximum expiry - PublicKey emptyLocalKey; PublicKey localCfgListed = randomNode(); - trustedKeys->load(emptyLocalKey, {toStr(localCfgListed)}, {}); + trustedKeys->load({}, {toStr(localCfgListed)}, {}); BEAST_EXPECT( trustedKeys->expires() && trustedKeys->expires().value() == NetClock::time_point::max()); @@ -1688,11 +1670,10 @@ class ValidatorList_test : public beast::unit_test::suite std::vector cfgPublishers( {strHex(publisherPublic)}); - PublicKey emptyLocalKey; std::vector emptyCfgKeys; - BEAST_EXPECT(trustedKeys->load( - emptyLocalKey, emptyCfgKeys, cfgPublishers)); + BEAST_EXPECT( + trustedKeys->load({}, emptyCfgKeys, cfgPublishers)); auto const version = 2; auto const sequence1 = 1; @@ -1795,7 +1776,6 @@ class ValidatorList_test : public beast::unit_test::suite { testcase("NegativeUNL"); jtx::Env env(*this); - PublicKey emptyLocalKey; ManifestCache manifests; auto createValidatorList = @@ -1820,7 +1800,7 @@ class ValidatorList_test : public beast::unit_test::suite cfgKeys.push_back(toBase58(TokenType::NodePublic, valKey)); activeValidators.emplace(calcNodeID(valKey)); } - if (trustedKeys->load(emptyLocalKey, cfgKeys, cfgPublishers)) + if (trustedKeys->load({}, cfgKeys, cfgPublishers)) { trustedKeys->updateTrusted( activeValidators, diff --git a/src/test/app/ValidatorSite_test.cpp b/src/test/app/ValidatorSite_test.cpp index 8ec86feadce..8acd2063358 100644 --- a/src/test/app/ValidatorSite_test.cpp +++ b/src/test/app/ValidatorSite_test.cpp @@ -172,7 +172,6 @@ class ValidatorSite_test : public beast::unit_test::suite test::StreamSink sink; beast::Journal journal{sink}; - PublicKey emptyLocalKey; std::vector emptyCfgKeys; struct publisher { @@ -229,8 +228,7 @@ class ValidatorSite_test : public beast::unit_test::suite item.uri = uri.str(); } - BEAST_EXPECT( - trustedKeys.load(emptyLocalKey, emptyCfgKeys, cfgPublishers)); + BEAST_EXPECT(trustedKeys.load({}, emptyCfgKeys, cfgPublishers)); // Normally, tests will only need a fraction of this time, // but sometimes DNS resolution takes an inordinate amount diff --git a/src/test/app/XChain_test.cpp b/src/test/app/XChain_test.cpp index e6e193adf10..e2816f06096 100644 --- a/src/test/app/XChain_test.cpp +++ b/src/test/app/XChain_test.cpp @@ -728,7 +728,7 @@ struct XChain_test : public beast::unit_test::suite, // Locking chain is XRP, // - Issuing chain is XRP with issuing chain is the root account. // --------------------------------------------------------------------- - Account a, b; + Account a("a"), b("b"); Issue ia, ib; std::tuple lcs{ @@ -5005,7 +5005,8 @@ struct XChainSim_test : public beast::unit_test::suite, // create 10 accounts + door funded on both chains, and store // in ChainStateTracker the initial amount of these accounts - Account doorXRPLocking, doorUSDLocking, doorUSDIssuing; + Account doorXRPLocking("doorXRPLocking"), + doorUSDLocking("doorUSDLocking"), doorUSDIssuing("doorUSDIssuing"); constexpr size_t num_acct = 10; auto a = [&doorXRPLocking, &doorUSDLocking, &doorUSDIssuing]() { diff --git a/src/test/consensus/NegativeUNL_test.cpp b/src/test/consensus/NegativeUNL_test.cpp index fee790281a0..8cbb57444bd 100644 --- a/src/test/consensus/NegativeUNL_test.cpp +++ b/src/test/consensus/NegativeUNL_test.cpp @@ -778,8 +778,10 @@ class NegativeUNLVoteInternal_test : public beast::unit_test::suite // one add, one remove auto txSet = std::make_shared( SHAMapType::TRANSACTION, env.app().getNodeFamily()); - PublicKey toDisableKey; - PublicKey toReEnableKey; + PublicKey toDisableKey( + derivePublicKey(KeyType::ed25519, randomSecretKey())); + PublicKey toReEnableKey( + derivePublicKey(KeyType::ed25519, randomSecretKey())); LedgerIndex seq(1234); BEAST_EXPECT(countTx(txSet) == 0); vote.addTx(seq, toDisableKey, NegativeUNLVote::ToDisable, txSet); diff --git a/src/test/jtx/Account.h b/src/test/jtx/Account.h index 1595d444354..cca92e76fe0 100644 --- a/src/test/jtx/Account.h +++ b/src/test/jtx/Account.h @@ -46,7 +46,7 @@ class Account /** The master account. */ static Account const master; - Account() = default; + Account() = delete; Account(Account&&) = default; Account(Account const&) = default; Account& diff --git a/src/test/jtx/Env_test.cpp b/src/test/jtx/Env_test.cpp index 6f09f49ed5d..154abe44ce5 100644 --- a/src/test/jtx/Env_test.cpp +++ b/src/test/jtx/Env_test.cpp @@ -50,7 +50,7 @@ class Env_test : public beast::unit_test::suite { using namespace jtx; { - Account a; + Account a("chenna"); Account b(a); a = b; a = std::move(b); diff --git a/src/test/overlay/reduce_relay_test.cpp b/src/test/overlay/reduce_relay_test.cpp index c722476dbaa..025e3295f2a 100644 --- a/src/test/overlay/reduce_relay_test.cpp +++ b/src/test/overlay/reduce_relay_test.cpp @@ -58,6 +58,12 @@ static constexpr std::uint32_t MAX_MESSAGES = 200000; class PeerPartial : public Peer { public: + PeerPartial() + : nodePublicKey_(derivePublicKey(KeyType::ed25519, randomSecretKey())) + { + } + + PublicKey nodePublicKey_; virtual ~PeerPartial() { } @@ -103,8 +109,7 @@ class PeerPartial : public Peer PublicKey const& getNodePublic() const override { - static PublicKey key{}; - return key; + return nodePublicKey_; } Json::Value json() override @@ -312,9 +317,8 @@ class Validator using Links = std::unordered_map; public: - Validator() + Validator() : pkey_(std::get<0>(randomKeyPair(KeyType::ed25519))) { - pkey_ = std::get<0>(randomKeyPair(KeyType::ed25519)); protocol::TMValidation v; v.set_validation("validation"); message_ = std::make_shared(v, protocol::mtVALIDATION, pkey_); @@ -439,7 +443,7 @@ class Validator private: Links links_; - PublicKey pkey_{}; + PublicKey pkey_; MessageSPtr message_ = nullptr; inline static std::uint16_t sid_ = 0; std::uint16_t id_ = 0; @@ -926,7 +930,7 @@ class reduce_relay_test : public beast::unit_test::suite bool isSelected_ = false; Peer::id_t peer_; std::uint16_t validator_; - PublicKey key_; + std::optional key_; time_point time_; bool handled_ = false; }; @@ -1052,17 +1056,17 @@ class reduce_relay_test : public beast::unit_test::suite // 4) peer is in Slot's peers_ (if not then it is deleted // by Slots::deleteIdlePeers()) bool mustHandle = false; - if (event.state_ == State::On) + if (event.state_ == State::On && BEAST_EXPECT(event.key_)) { event.isSelected_ = - network_.overlay().isSelected(event.key_, event.peer_); - auto peers = network_.overlay().getPeers(event.key_); + network_.overlay().isSelected(*event.key_, event.peer_); + auto peers = network_.overlay().getPeers(*event.key_); auto d = reduce_relay::epoch(now).count() - std::get<3>(peers[event.peer_]); mustHandle = event.isSelected_ && d > milliseconds(reduce_relay::IDLED).count() && network_.overlay().inState( - event.key_, reduce_relay::PeerState::Squelched) > + *event.key_, reduce_relay::PeerState::Squelched) > 0 && peers.find(event.peer_) != peers.end(); } diff --git a/src/test/protocol/PublicKey_test.cpp b/src/test/protocol/PublicKey_test.cpp index c7974021877..040d752f481 100644 --- a/src/test/protocol/PublicKey_test.cpp +++ b/src/test/protocol/PublicKey_test.cpp @@ -363,10 +363,12 @@ class PublicKey_test : public beast::unit_test::suite } // Try some random secret keys - std::array keys; + std::vector keys; + keys.reserve(32); - for (std::size_t i = 0; i != keys.size(); ++i) - keys[i] = derivePublicKey(keyType, randomSecretKey()); + for (std::size_t i = 0; i != keys.capacity(); ++i) + keys.emplace_back(derivePublicKey(keyType, randomSecretKey())); + BEAST_EXPECT(keys.size() == 32); for (std::size_t i = 0; i != keys.size(); ++i) { @@ -447,7 +449,11 @@ class PublicKey_test : public beast::unit_test::suite BEAST_EXPECT(pk1 == pk2); BEAST_EXPECT(pk2 == pk1); - PublicKey pk3; + PublicKey pk3 = derivePublicKey( + KeyType::secp256k1, + generateSecretKey( + KeyType::secp256k1, generateSeed("arbitraryPassPhrase"))); + // Testing the copy assignment operation of PublicKey class pk3 = pk2; BEAST_EXPECT(pk3 == pk2); BEAST_EXPECT(pk1 == pk3); diff --git a/src/test/protocol/STObject_test.cpp b/src/test/protocol/STObject_test.cpp index d89916eddf7..34d1cc82fac 100644 --- a/src/test/protocol/STObject_test.cpp +++ b/src/test/protocol/STObject_test.cpp @@ -17,7 +17,6 @@ */ //============================================================================== -#include #include #include #include @@ -609,12 +608,7 @@ class STObject_test : public beast::unit_test::suite auto const kp = generateKeyPair(KeyType::secp256k1, generateSeed("masterpassphrase")); st[sf5] = kp.first; - BEAST_EXPECT(st[sf5] != PublicKey{}); st[~sf5] = std::nullopt; -#if 0 - pk = st[sf5]; - BEAST_EXPECT(pk.size() == 0); -#endif } // By reference fields diff --git a/src/test/protocol/SecretKey_test.cpp b/src/test/protocol/SecretKey_test.cpp index 25b5511dd74..08a19124508 100644 --- a/src/test/protocol/SecretKey_test.cpp +++ b/src/test/protocol/SecretKey_test.cpp @@ -275,10 +275,12 @@ class SecretKey_test : public beast::unit_test::suite } // Try some random secret keys - std::array keys; + std::vector keys; + keys.reserve(32); - for (std::size_t i = 0; i != keys.size(); ++i) - keys[i] = randomSecretKey(); + for (std::size_t i = 0; i != keys.capacity(); ++i) + keys.emplace_back(randomSecretKey()); + BEAST_EXPECT(keys.size() == 32); for (std::size_t i = 0; i != keys.size(); ++i) { diff --git a/src/test/rpc/KeyGeneration_test.cpp b/src/test/rpc/KeyGeneration_test.cpp index 735bae40bd8..28f9afd3b7f 100644 --- a/src/test/rpc/KeyGeneration_test.cpp +++ b/src/test/rpc/KeyGeneration_test.cpp @@ -16,8 +16,6 @@ OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ //============================================================================== - -#include #include #include #include @@ -337,8 +335,11 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(!contains_error(error)); - BEAST_EXPECT(ret.first.size() != 0); - BEAST_EXPECT(ret.first == publicKey); + if (BEAST_EXPECT(ret)) + { + BEAST_EXPECT(ret->first.size() != 0); + BEAST_EXPECT(ret->first == publicKey); + } } { @@ -348,8 +349,11 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(!contains_error(error)); - BEAST_EXPECT(ret.first.size() != 0); - BEAST_EXPECT(ret.first == publicKey); + if (BEAST_EXPECT(ret)) + { + BEAST_EXPECT(ret->first.size() != 0); + BEAST_EXPECT(ret->first == publicKey); + } } { @@ -359,8 +363,11 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(!contains_error(error)); - BEAST_EXPECT(ret.first.size() != 0); - BEAST_EXPECT(ret.first == publicKey); + if (BEAST_EXPECT(ret)) + { + BEAST_EXPECT(ret->first.size() != 0); + BEAST_EXPECT(ret->first == publicKey); + } } keyType.emplace("secp256k1"); @@ -375,8 +382,11 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(!contains_error(error)); - BEAST_EXPECT(ret.first.size() != 0); - BEAST_EXPECT(ret.first == publicKey); + if (BEAST_EXPECT(ret)) + { + BEAST_EXPECT(ret->first.size() != 0); + BEAST_EXPECT(ret->first == publicKey); + } } { @@ -388,8 +398,11 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(!contains_error(error)); - BEAST_EXPECT(ret.first.size() != 0); - BEAST_EXPECT(ret.first == publicKey); + if (BEAST_EXPECT(ret)) + { + BEAST_EXPECT(ret->first.size() != 0); + BEAST_EXPECT(ret->first == publicKey); + } } { @@ -401,8 +414,11 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(!contains_error(error)); - BEAST_EXPECT(ret.first.size() != 0); - BEAST_EXPECT(ret.first == publicKey); + if (BEAST_EXPECT(ret)) + { + BEAST_EXPECT(ret->first.size() != 0); + BEAST_EXPECT(ret->first == publicKey); + } } } @@ -416,10 +432,10 @@ class WalletPropose_test : public ripple::TestSuite params[jss::secret] = 314159265; auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'secret', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { @@ -430,10 +446,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'secret', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { @@ -445,7 +461,7 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); - BEAST_EXPECT(ret.first.size() == 0); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'secret', not string."); @@ -460,10 +476,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "The secret field is not allowed if key_type is used."); - BEAST_EXPECT(ret.first.size() == 0); } // Specify unknown or bad "key_type" @@ -475,9 +491,9 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'key_type'."); - BEAST_EXPECT(ret.first.size() == 0); } { @@ -488,10 +504,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'key_type', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { @@ -502,10 +518,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'key_type', not string."); - BEAST_EXPECT(ret.first.size() == 0); } // Specify non-string passphrase @@ -517,10 +533,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'passphrase', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a passphrase: object @@ -531,10 +547,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'passphrase', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a passphrase: array @@ -545,10 +561,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'passphrase', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a passphrase: empty string @@ -559,8 +575,8 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT(error[jss::error_message] == "Disallowed seed."); - BEAST_EXPECT(ret.first.size() == 0); } // Specify non-string or invalid seed @@ -572,10 +588,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'seed', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a string: object @@ -586,10 +602,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'seed', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a string: array @@ -600,10 +616,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'seed', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a seed: empty @@ -614,8 +630,8 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT(error[jss::error_message] == "Disallowed seed."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a seed: invalid characters @@ -626,8 +642,8 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT(error[jss::error_message] == "Disallowed seed."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a seed: random string @@ -638,8 +654,8 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT(error[jss::error_message] == "Disallowed seed."); - BEAST_EXPECT(ret.first.size() == 0); } // Specify non-string or invalid seed_hex @@ -651,10 +667,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'seed_hex', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a string: object @@ -665,10 +681,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'seed_hex', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // not a string: array @@ -679,10 +695,10 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT( error[jss::error_message] == "Invalid field 'seed_hex', not string."); - BEAST_EXPECT(ret.first.size() == 0); } { // empty @@ -693,8 +709,8 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT(error[jss::error_message] == "Disallowed seed."); - BEAST_EXPECT(ret.first.size() == 0); } { // short @@ -705,8 +721,8 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT(error[jss::error_message] == "Disallowed seed."); - BEAST_EXPECT(ret.first.size() == 0); } { // not hex @@ -717,8 +733,8 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT(error[jss::error_message] == "Disallowed seed."); - BEAST_EXPECT(ret.first.size() == 0); } { // overlong @@ -730,8 +746,8 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(contains_error(error)); + BEAST_EXPECT(!ret); BEAST_EXPECT(error[jss::error_message] == "Disallowed seed."); - BEAST_EXPECT(ret.first.size() == 0); } } @@ -750,8 +766,11 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(!contains_error(error)); - BEAST_EXPECT(ret.first.size() != 0); - BEAST_EXPECT(toBase58(calcAccountID(ret.first)) == addr); + if (BEAST_EXPECT(ret)) + { + BEAST_EXPECT(ret->first.size() != 0); + BEAST_EXPECT(toBase58(calcAccountID(ret->first)) == addr); + } } { @@ -779,8 +798,11 @@ class WalletPropose_test : public ripple::TestSuite auto ret = keypairForSignature(params, error); BEAST_EXPECT(!contains_error(error)); - BEAST_EXPECT(ret.first.size() != 0); - BEAST_EXPECT(toBase58(calcAccountID(ret.first)) == addr); + if (BEAST_EXPECT(ret)) + { + BEAST_EXPECT(ret->first.size() != 0); + BEAST_EXPECT(toBase58(calcAccountID(ret->first)) == addr); + } } {