ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-01 00:15:51 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor: Retire RequireFullyCanonicalSig amendment (#6035)

Browse Source 
Amendments activated for more than 2 years can be retired. This change retires the RequireFullyCanonicalSig amendment.
This commit is contained in:
Jingchen
2025-11-24 13:58:37 +00:00
committed by GitHub
parent a791c03dc1
commit 21c02232a5
14 changed files with 51 additions and 146 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
include/xrpl/protocol/PublicKey.h
View File

@@ -231,11 +231,7 @@ verifyDigest(
SHA512-Half, and the resulting digest is signed.
*/
[[nodiscard]] bool
verify(
PublicKey const& publicKey,
Slice const& m,
Slice const& sig,
bool mustBeFullyCanonical = true) noexcept;
verify(PublicKey const& publicKey, Slice const& m, Slice const& sig) noexcept;
/** Calculate the 160-bit node ID from a node public key. */
NodeID

36
include/xrpl/protocol/STTx.h
View File

@@ -103,22 +103,15 @@ public:
std::optional<std::reference_wrapper<SField const>> signatureTarget =
{});
enum class RequireFullyCanonicalSig : bool { no, yes };
/** Check the signature.
@param requireCanonicalSig If `true`, check that the signature is fully
canonical. If `false`, only check that the signature is valid.
@param rules The current ledger rules.
@return `true` if valid signature. If invalid, the error message string.
*/
Expected<void, std::string>
checkSign(RequireFullyCanonicalSig requireCanonicalSig, Rules const& rules)
const;
checkSign(Rules const& rules) const;
Expected<void, std::string>
checkBatchSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const;
checkBatchSign(Rules const& rules) const;
// SQL Functions with metadata.
static std::string const&
@@ -140,40 +133,25 @@ public:
private:
/** Check the signature.
@param requireCanonicalSig If `true`, check that the signature is fully
canonical. If `false`, only check that the signature is valid.
@param rules The current ledger rules.
@param sigObject Reference to object that contains the signature fields.
Will be *this more often than not.
@return `true` if valid signature. If invalid, the error message string.
*/
Expected<void, std::string>
checkSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules,
STObject const& sigObject) const;
checkSign(Rules const& rules, STObject const& sigObject) const;
Expected<void, std::string>
checkSingleSign(
RequireFullyCanonicalSig requireCanonicalSig,
STObject const& sigObject) const;
checkSingleSign(STObject const& sigObject) const;
Expected<void, std::string>
checkMultiSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules,
STObject const& sigObject) const;
checkMultiSign(Rules const& rules, STObject const& sigObject) const;
Expected<void, std::string>
checkBatchSingleSign(
STObject const& batchSigner,
RequireFullyCanonicalSig requireCanonicalSig) const;
checkBatchSingleSign(STObject const& batchSigner) const;
Expected<void, std::string>
checkBatchMultiSign(
STObject const& batchSigner,
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const;
checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const;
STBase*
copy(std::size_t n, void* buf) const override;

2
include/xrpl/protocol/detail/features.macro
View File

@@ -65,7 +65,6 @@ XRPL_FIX (UniversalNumber, Supported::yes, VoteBehavior::DefaultNo
XRPL_FEATURE(XRPFees, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (RemoveNFTokenAutoTrustLine, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(FlowSortStrands, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(RequireFullyCanonicalSig, Supported::yes, VoteBehavior::DefaultYes)
XRPL_FEATURE(Flow, Supported::yes, VoteBehavior::DefaultYes)
// The following amendments are obsolete, but must remain supported
@@ -133,6 +132,7 @@ XRPL_RETIRE_FEATURE(MultiSignReserve)
XRPL_RETIRE_FEATURE(NegativeUNL)
XRPL_RETIRE_FEATURE(NonFungibleTokensV1_1)
XRPL_RETIRE_FEATURE(PayChan)
XRPL_RETIRE_FEATURE(RequireFullyCanonicalSig)
XRPL_RETIRE_FEATURE(SortedDirectories)
XRPL_RETIRE_FEATURE(TicketBatch)
XRPL_RETIRE_FEATURE(TickSize)

9
src/libxrpl/protocol/PublicKey.cpp
View File

@@ -267,18 +267,13 @@ verifyDigest(
}
bool
verify(
PublicKey const& publicKey,
Slice const& m,
Slice const& sig,
bool mustBeFullyCanonical) noexcept
verify(PublicKey const& publicKey, Slice const& m, Slice const& sig) noexcept
{
if (auto const type = publicKeyType(publicKey))
{
if (*type == KeyType::secp256k1)
{
return verifyDigest(
publicKey, sha512Half(m), sig, mustBeFullyCanonical);
return verifyDigest(publicKey, sha512Half(m), sig);
}
else if (*type == KeyType::ed25519)
{

76
src/libxrpl/protocol/STTx.cpp
View File

@@ -237,10 +237,7 @@ STTx::sign(
}
Expected<void, std::string>
STTx::checkSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules,
STObject const& sigObject) const
STTx::checkSign(Rules const& rules, STObject const& sigObject) const
{
try
{
@@ -249,9 +246,8 @@ STTx::checkSign(
// multi-signing. Otherwise we're single-signing.
Blob const& signingPubKey = sigObject.getFieldVL(sfSigningPubKey);
return signingPubKey.empty()
? checkMultiSign(requireCanonicalSig, rules, sigObject)
: checkSingleSign(requireCanonicalSig, sigObject);
return signingPubKey.empty() ? checkMultiSign(rules, sigObject)
: checkSingleSign(sigObject);
}
catch (std::exception const&)
{
@@ -260,18 +256,16 @@ STTx::checkSign(
}
Expected<void, std::string>
STTx::checkSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const
STTx::checkSign(Rules const& rules) const
{
if (auto const ret = checkSign(requireCanonicalSig, rules, *this); !ret)
if (auto const ret = checkSign(rules, *this); !ret)
return ret;
/* Placeholder for field that will be added by Lending Protocol
if (isFieldPresent(sfCounterpartySignature))
{
auto const counterSig = getFieldObject(sfCounterpartySignature);
if (auto const ret = checkSign(requireCanonicalSig, rules, counterSig);
if (auto const ret = checkSign(rules, counterSig);
!ret)
return Unexpected("Counterparty: " + ret.error());
}
@@ -280,9 +274,7 @@ STTx::checkSign(
}
Expected<void, std::string>
STTx::checkBatchSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const
STTx::checkBatchSign(Rules const& rules) const
{
try
{
@@ -299,8 +291,8 @@ STTx::checkBatchSign(
{
Blob const& signingPubKey = signer.getFieldVL(sfSigningPubKey);
auto const result = signingPubKey.empty()
? checkBatchMultiSign(signer, requireCanonicalSig, rules)
: checkBatchSingleSign(signer, requireCanonicalSig);
? checkBatchMultiSign(signer, rules)
: checkBatchSingleSign(signer);
if (!result)
return result;
@@ -395,10 +387,7 @@ STTx::getMetaSQL(
}
static Expected<void, std::string>
singleSignHelper(
STObject const& sigObject,
Slice const& data,
bool const fullyCanonical)
singleSignHelper(STObject const& sigObject, Slice const& data)
{
// We don't allow both a non-empty sfSigningPubKey and an sfSigners.
// That would allow the transaction to be signed two ways. So if both
@@ -413,11 +402,8 @@ singleSignHelper(
if (publicKeyType(makeSlice(spk)))
{
Blob const signature = sigObject.getFieldVL(sfTxnSignature);
validSig = verify(
PublicKey(makeSlice(spk)),
data,
makeSlice(signature),
fullyCanonical);
validSig =
verify(PublicKey(makeSlice(spk)), data, makeSlice(signature));
}
}
catch (std::exception const&)
@@ -432,33 +418,24 @@ singleSignHelper(
}
Expected<void, std::string>
STTx::checkSingleSign(
RequireFullyCanonicalSig requireCanonicalSig,
STObject const& sigObject) const
STTx::checkSingleSign(STObject const& sigObject) const
{
auto const data = getSigningData(*this);
bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) ||
(requireCanonicalSig == STTx::RequireFullyCanonicalSig::yes);
return singleSignHelper(sigObject, makeSlice(data), fullyCanonical);
return singleSignHelper(sigObject, makeSlice(data));
}
Expected<void, std::string>
STTx::checkBatchSingleSign(
STObject const& batchSigner,
RequireFullyCanonicalSig requireCanonicalSig) const
STTx::checkBatchSingleSign(STObject const& batchSigner) const
{
Serializer msg;
serializeBatch(msg, getFlags(), getBatchTransactionIDs());
bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) ||
(requireCanonicalSig == STTx::RequireFullyCanonicalSig::yes);
return singleSignHelper(batchSigner, msg.slice(), fullyCanonical);
return singleSignHelper(batchSigner, msg.slice());
}
Expected<void, std::string>
multiSignHelper(
STObject const& sigObject,
std::optional<AccountID> txnAccountID,
bool const fullyCanonical,
std::function<Serializer(AccountID const&)> makeMsg,
Rules const& rules)
{
@@ -515,8 +492,7 @@ multiSignHelper(
validSig = verify(
PublicKey(makeSlice(spk)),
makeMsg(accountID).slice(),
makeSlice(signature),
fullyCanonical);
makeSlice(signature));
}
}
catch (std::exception const& e)
@@ -535,14 +511,8 @@ multiSignHelper(
}
Expected<void, std::string>
STTx::checkBatchMultiSign(
STObject const& batchSigner,
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules) const
STTx::checkBatchMultiSign(STObject const& batchSigner, Rules const& rules) const
{
bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) ||
(requireCanonicalSig == RequireFullyCanonicalSig::yes);
// We can ease the computational load inside the loop a bit by
// pre-constructing part of the data that we hash. Fill a Serializer
// with the stuff that stays constant from signature to signature.
@@ -551,7 +521,6 @@ STTx::checkBatchMultiSign(
return multiSignHelper(
batchSigner,
std::nullopt,
fullyCanonical,
[&dataStart](AccountID const& accountID) -> Serializer {
Serializer s = dataStart;
finishMultiSigningData(accountID, s);
@@ -561,14 +530,8 @@ STTx::checkBatchMultiSign(
}
Expected<void, std::string>
STTx::checkMultiSign(
RequireFullyCanonicalSig requireCanonicalSig,
Rules const& rules,
STObject const& sigObject) const
STTx::checkMultiSign(Rules const& rules, STObject const& sigObject) const
{
bool const fullyCanonical = (getFlags() & tfFullyCanonicalSig) ||
(requireCanonicalSig == RequireFullyCanonicalSig::yes);
// Used inside the loop in multiSignHelper to enforce that
// the account owner may not multisign for themselves.
auto const txnAccountID = &sigObject != this
@@ -582,7 +545,6 @@ STTx::checkMultiSign(
return multiSignHelper(
sigObject,
txnAccountID,
fullyCanonical,
[&dataStart](AccountID const& accountID) -> Serializer {
Serializer s = dataStart;
finishMultiSigningData(accountID, s);

17
src/test/app/tx/apply_test.cpp
View File

@@ -35,23 +35,6 @@ public:
SerialIter sitTrans(makeSlice(*ret));
STTx const tx = *std::make_shared<STTx const>(std::ref(sitTrans));
{
test::jtx::Env no_fully_canonical(
*this,
test::jtx::testable_amendments() -
featureRequireFullyCanonicalSig);
Validity valid = checkValidity(
no_fully_canonical.app().getHashRouter(),
tx,
no_fully_canonical.current()->rules(),
no_fully_canonical.app().config())
.first;
if (valid != Validity::Valid)
fail("Non-Fully canonical signature was not permitted");
}
{
test::jtx::Env fully_canonical(
*this, test::jtx::testable_amendments());

6
src/test/ledger/View_test.cpp
View File

@@ -1114,10 +1114,10 @@ class GetAmendments_test : public beast::unit_test::suite
break;
}
// There should be at least 5 amendments. Don't do exact comparison
// There should be at least 3 amendments. Don't do exact comparison
// to avoid maintenance as more amendments are added in the future.
BEAST_EXPECT(i == 254);
BEAST_EXPECT(majorities.size() >= 5);
BEAST_EXPECT(majorities.size() >= 3);
// None of the amendments should be enabled yet.
auto enableds = getEnabledAmendments(*env.closed());
@@ -1135,7 +1135,7 @@ class GetAmendments_test : public beast::unit_test::suite
break;
}
BEAST_EXPECT(i == 255);
BEAST_EXPECT(enableds.size() >= 5);
BEAST_EXPECT(enableds.size() >= 3);
}
void

3
src/test/protocol/STTx_test.cpp
View File

@@ -1615,8 +1615,7 @@ public:
BEAST_EXPECT(!defaultRules.enabled(featureAMM));
unexpected(
!j.checkSign(STTx::RequireFullyCanonicalSig::yes, defaultRules),
"Transaction fails signature test");
!j.checkSign(defaultRules), "Transaction fails signature test");
Serializer rawTxn;
j.add(rawTxn);

8
src/test/protocol/SecretKey_test.cpp
View File

@@ -145,7 +145,7 @@ public:
auto sig = sign(pk, sk, makeSlice(data));
BEAST_EXPECT(sig.size() != 0);
BEAST_EXPECT(verify(pk, makeSlice(data), sig, true));
BEAST_EXPECT(verify(pk, makeSlice(data), sig));
// Construct wrong data:
auto badData = data;
@@ -156,17 +156,17 @@ public:
std::max_element(badData.begin(), badData.end()));
// Wrong data: should fail
BEAST_EXPECT(!verify(pk, makeSlice(badData), sig, true));
BEAST_EXPECT(!verify(pk, makeSlice(badData), sig));
// Slightly change the signature:
if (auto ptr = sig.data())
ptr[j % sig.size()]++;
// Wrong signature: should fail
BEAST_EXPECT(!verify(pk, makeSlice(data), sig, true));
BEAST_EXPECT(!verify(pk, makeSlice(data), sig));
// Wrong data and signature: should fail
BEAST_EXPECT(!verify(pk, makeSlice(badData), sig, true));
BEAST_EXPECT(!verify(pk, makeSlice(badData), sig));
}
}
}

18
src/test/rpc/Feature_test.cpp
View File

@@ -183,16 +183,16 @@ class Feature_test : public beast::unit_test::suite
using namespace test::jtx;
Env env{*this};
auto jrr = env.rpc("feature", "RequireFullyCanonicalSig")[jss::result];
auto jrr = env.rpc("feature", "Flow")[jss::result];
BEAST_EXPECTS(jrr[jss::status] == jss::success, "status");
jrr.removeMember(jss::status);
BEAST_EXPECT(jrr.size() == 1);
BEAST_EXPECT(
jrr.isMember("00C1FC4A53E60AB02C864641002B3172F38677E29C26C54066851"
"79B37E1EDAC"));
jrr.isMember("740352F2412A9909880C23A559FCECEDA3BE2126FED62FC7660D6"
"28A06927F11"));
auto feature = *(jrr.begin());
BEAST_EXPECTS(feature[jss::name] == "RequireFullyCanonicalSig", "name");
BEAST_EXPECTS(feature[jss::name] == "Flow", "name");
BEAST_EXPECTS(!feature[jss::enabled].asBool(), "enabled");
BEAST_EXPECTS(
feature[jss::vetoed].isBool() && !feature[jss::vetoed].asBool(),
@@ -200,7 +200,7 @@ class Feature_test : public beast::unit_test::suite
BEAST_EXPECTS(feature[jss::supported].asBool(), "supported");
// feature names are case-sensitive - expect error here
jrr = env.rpc("feature", "requireFullyCanonicalSig")[jss::result];
jrr = env.rpc("feature", "flow")[jss::result];
BEAST_EXPECT(jrr[jss::error] == "badFeature");
BEAST_EXPECT(jrr[jss::error_message] == "Feature unknown or invalid.");
}
@@ -419,9 +419,9 @@ class Feature_test : public beast::unit_test::suite
break;
}
// There should be at least 5 amendments. Don't do exact comparison
// There should be at least 3 amendments. Don't do exact comparison
// to avoid maintenance as more amendments are added in the future.
BEAST_EXPECT(majorities.size() >= 5);
BEAST_EXPECT(majorities.size() >= 3);
std::map<std::string, VoteBehavior> const& votes =
ripple::detail::supportedAmendments();
@@ -476,8 +476,8 @@ class Feature_test : public beast::unit_test::suite
testcase("Veto");
using namespace test::jtx;
Env env{*this, FeatureBitset{featureRequireFullyCanonicalSig}};
constexpr char const* featureName = "RequireFullyCanonicalSig";
Env env{*this, FeatureBitset{featureFlow}};
constexpr char const* featureName = "Flow";
auto jrr = env.rpc("feature", featureName)[jss::result];
if (!BEAST_EXPECTS(jrr[jss::status] == jss::success, "status"))

3
src/xrpld/app/tx/detail/Batch.cpp
View File

@@ -451,8 +451,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
}
// Check the batch signers signatures.
auto const sigResult = ctx.tx.checkBatchSign(
STTx::RequireFullyCanonicalSig::yes, ctx.rules);
auto const sigResult = ctx.tx.checkBatchSign(ctx.rules);
if (!sigResult)
{

2
src/xrpld/app/tx/detail/PayChan.cpp
View File

@@ -440,7 +440,7 @@ PayChanClaim::preflight(PreflightContext const& ctx)
PublicKey const pk(ctx.tx[sfPublicKey]);
Serializer msg;
serializePayChanAuthorization(msg, k.key, authAmt);
if (!verify(pk, msg.slice(), *sig, /*canonical*/ true))
if (!verify(pk, msg.slice(), *sig))
return temBAD_SIGNATURE;
}

8
src/xrpld/app/tx/detail/apply.cpp
View File

@@ -58,13 +58,7 @@ checkValidity(
if (!any(flags & SF_SIGGOOD))
{
// Don't know signature state. Check it.
auto const requireCanonicalSig =
rules.enabled(featureRequireFullyCanonicalSig)
? STTx::RequireFullyCanonicalSig::yes
: STTx::RequireFullyCanonicalSig::no;
auto const sigVerify = tx.checkSign(requireCanonicalSig, rules);
auto const sigVerify = tx.checkSign(rules);
if (!sigVerify)
{
router.setFlags(id, SF_SIGBAD);

3
src/xrpld/rpc/handlers/PayChanClaim.cpp
View File

@@ -137,8 +137,7 @@ doChannelVerify(RPC::JsonContext& context)
serializePayChanAuthorization(msg, channelId, XRPAmount(drops));
Json::Value result;
result[jss::signature_verified] =
verify(*pk, msg.slice(), makeSlice(*sig), /*canonical*/ true);
result[jss::signature_verified] = verify(*pk, msg.slice(), makeSlice(*sig));
return result;
}