Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Work in Progress] add AccountPermission #5164

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion include/xrpl/protocol/Feature.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace detail {
// Feature.cpp. Because it's only used to reserve storage, and determine how
// large to make the FeatureBitset, it MAY be larger. It MUST NOT be less than
// the actual number of amendments. A LogicError on startup will verify this.
static constexpr std::size_t numFeatures = 83;
static constexpr std::size_t numFeatures = 84;

/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
Expand Down
11 changes: 11 additions & 0 deletions include/xrpl/protocol/Indexes.h
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,17 @@ amm(Issue const& issue1, Issue const& issue2) noexcept;
Keylet
amm(uint256 const& amm) noexcept;

/** An AccountPermission */
/** @{ */
Keylet
accountPermission(
AccountID const& account,
AccountID const& authorizedAccount) noexcept;

Keylet
accountPermission(uint256 const& key) noexcept;
/** @} */

Keylet
bridge(STXChainBridge const& bridge, STXChainBridge::ChainType chainType);

Expand Down
88 changes: 88 additions & 0 deletions include/xrpl/protocol/Permissions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2024 Ripple Labs Inc.
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================

#ifndef RIPPLE_PROTOCOL_PERMISSION_H_INCLUDED
#define RIPPLE_PROTOCOL_PERMISSION_H_INCLUDED

#include <xrpl/protocol/STTx.h>
#include <optional>
#include <string>
#include <unordered_map>

namespace ripple {

/**
* We have transaction type permissions and granular type
* permissions. Since we will reuse the TransactionFormats to parse the
* Transaction Permissions, we only define the GranularPermissionType here.
*/

enum GranularPermissionType : std::uint32_t {
gpTrustlineAuthorize = 65537,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is gp prefix necessary? Can we just have TrustlineAuthorize, etc?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gp is to indicate it is a granular permission set's member. I'm ok with either way.


gpTrustlineFreeze = 65538,

gpTrustlineUnfreeze = 65539,

gpAccountDomainSet = 65540,

gpAccountEmailHashSet = 65541,

gpAccountMessageKeySet = 65542,

gpAccountTransferRateSet = 65543,

gpAccountTickSizeSet = 65544,

gpPaymentMint = 65545,

gpPaymentBurn = 65546,

gpMPTokenIssuanceLock = 65547,

gpMPTokenIssuanceUnlock = 65548,
};

class Permission
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should probably delete copy and copy assignment constructors

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this still needs to be done?

{
private:
Permission();

std::unordered_map<std::string, GranularPermissionType>
granularPermissionMap;

std::unordered_map<GranularPermissionType, TxType> granularTxTypeMap;

public:
static Permission const&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just making all members as static? Don't need to get an instance then.

getInstance();

std::optional<std::uint32_t>
getGranularValue(std::string const& name) const;

std::optional<TxType>
getGranularTxType(GranularPermissionType const& gpType) const;

bool
isProhibited(std::string const& name) const;
};

} // namespace ripple

#endif
1 change: 1 addition & 0 deletions include/xrpl/protocol/detail/features.macro
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
// If you add an amendment here, then do not forget to increment `numFeatures`
// in include/xrpl/protocol/Feature.h.

XRPL_FEATURE(AccountPermission, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(Credentials, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FEATURE(AMMClawback, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (AMMv1_2, Supported::yes, VoteBehavior::DefaultNo)
Expand Down
13 changes: 13 additions & 0 deletions include/xrpl/protocol/detail/ledger_entries.macro
Original file line number Diff line number Diff line change
Expand Up @@ -436,3 +436,16 @@ LEDGER_ENTRY(ltCREDENTIAL, 0x0081, Credential, ({
{sfPreviousTxnID, soeREQUIRED},
{sfPreviousTxnLgrSeq, soeREQUIRED},
}))

/** A ledger object representing permissions an account has delegated to another account.

\sa keylet::accountPermission
*/
LEDGER_ENTRY(ltACCOUNT_PERMISSION, 0x0082, AccountPermission, ({
{sfAccount, soeREQUIRED},
{sfAuthorize, soeREQUIRED},
{sfPermissions, soeREQUIRED},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to make this optional since it could be empty?

Copy link
Collaborator Author

@yinyiqian1 yinyiqian1 Dec 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since being empty means deleting all the permissions, can we keep it as required? So that the user won't delete the permissions by accident when they mistakenly leave out the Permissions field. Giving an empty field is more clear that they want to delete all the permissions.

{sfOwnerNode, soeREQUIRED},
{sfPreviousTxnID, soeREQUIRED},
{sfPreviousTxnLgrSeq, soeREQUIRED},
}))
4 changes: 4 additions & 0 deletions include/xrpl/protocol/detail/sfields.macro
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ TYPED_SFIELD(sfEmitGeneration, UINT32, 46)
TYPED_SFIELD(sfVoteWeight, UINT32, 48)
TYPED_SFIELD(sfFirstNFTokenSequence, UINT32, 50)
TYPED_SFIELD(sfOracleDocumentID, UINT32, 51)
TYPED_SFIELD(sfPermissionValue, UINT32, 52)

// 64-bit integers (common)
TYPED_SFIELD(sfIndexNext, UINT64, 1)
Expand Down Expand Up @@ -274,6 +275,7 @@ TYPED_SFIELD(sfRegularKey, ACCOUNT, 8)
TYPED_SFIELD(sfNFTokenMinter, ACCOUNT, 9)
TYPED_SFIELD(sfEmitCallback, ACCOUNT, 10)
TYPED_SFIELD(sfHolder, ACCOUNT, 11)
TYPED_SFIELD(sfOnBehalfOf, ACCOUNT, 12)

// account (uncommon)
TYPED_SFIELD(sfHookAccount, ACCOUNT, 16)
Expand Down Expand Up @@ -323,6 +325,7 @@ UNTYPED_SFIELD(sfSignerEntry, OBJECT, 11)
UNTYPED_SFIELD(sfNFToken, OBJECT, 12)
UNTYPED_SFIELD(sfEmitDetails, OBJECT, 13)
UNTYPED_SFIELD(sfHook, OBJECT, 14)
UNTYPED_SFIELD(sfPermission, OBJECT, 15)

// inner object (uncommon)
UNTYPED_SFIELD(sfSigner, OBJECT, 16)
Expand Down Expand Up @@ -372,3 +375,4 @@ UNTYPED_SFIELD(sfPriceDataSeries, ARRAY, 24)
UNTYPED_SFIELD(sfAuthAccounts, ARRAY, 25)
UNTYPED_SFIELD(sfAuthorizeCredentials, ARRAY, 26)
UNTYPED_SFIELD(sfUnauthorizeCredentials, ARRAY, 27)
UNTYPED_SFIELD(sfPermissions, ARRAY, 28)
6 changes: 6 additions & 0 deletions include/xrpl/protocol/detail/transactions.macro
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,12 @@ TRANSACTION(ttCREDENTIAL_DELETE, 60, CredentialDelete, ({
{sfCredentialType, soeREQUIRED},
}))

/** This transaction type delegates authorized account specified permissions */
TRANSACTION(ttACCOUNT_PERMISSION_SET, 61, AccountPermissionSet, ({
{sfAuthorize, soeREQUIRED},
{sfPermissions, soeREQUIRED},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to make it optional since it could be empty?

}))


/** This system-generated transaction type is used to update the status of the various amendments.

Expand Down
2 changes: 2 additions & 0 deletions include/xrpl/protocol/jss.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ namespace jss {
JSS(AL_size); // out: GetCounts
JSS(AL_hit_rate); // out: GetCounts
JSS(Account); // in: TransactionSign; field.
JSS(AccountPermission); // ledger type.
JSS(AccountRoot); // ledger type.
JSS(AMM); // ledger type
JSS(AMMID); // field
Expand Down Expand Up @@ -132,6 +133,7 @@ JSS(account_hash); // out: LedgerToJson
JSS(account_id); // out: WalletPropose
JSS(account_nfts); // out: AccountNFTs
JSS(account_objects); // out: AccountObjects
JSS(account_permission); // in: AccountPermission
JSS(account_root); // in: LedgerEntry
JSS(account_sequence_next); // out: SubmitTransaction
JSS(account_sequence_available); // out: SubmitTransaction
Expand Down
18 changes: 18 additions & 0 deletions src/libxrpl/protocol/Indexes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
MPTOKEN_ISSUANCE = '~',
MPTOKEN = 't',
CREDENTIAL = 'D',
ACCOUNT_PERMISSION = 'P',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is already used by DEPOSIT_PREAUTH_CREDENTIALS


// No longer used or supported. Left here to reserve the space
// to avoid accidental reuse.
Expand Down Expand Up @@ -428,6 +429,23 @@
return {ltAMM, id};
}

Keylet
accountPermission(

Check warning on line 433 in src/libxrpl/protocol/Indexes.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Indexes.cpp#L433

Added line #L433 was not covered by tests
AccountID const& account,
AccountID const& authorizedAccount) noexcept
{
return {
ltACCOUNT_PERMISSION,
indexHash(
LedgerNameSpace::ACCOUNT_PERMISSION, account, authorizedAccount)};

Check warning on line 440 in src/libxrpl/protocol/Indexes.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Indexes.cpp#L439-L440

Added lines #L439 - L440 were not covered by tests
}

Keylet
accountPermission(uint256 const& key) noexcept

Check warning on line 444 in src/libxrpl/protocol/Indexes.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Indexes.cpp#L444

Added line #L444 was not covered by tests
{
return {ltACCOUNT_PERMISSION, key};

Check warning on line 446 in src/libxrpl/protocol/Indexes.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Indexes.cpp#L446

Added line #L446 was not covered by tests
}

Keylet
bridge(STXChainBridge const& bridge, STXChainBridge::ChainType chainType)
{
Expand Down
4 changes: 4 additions & 0 deletions src/libxrpl/protocol/InnerObjectFormats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ InnerObjectFormats::InnerObjectFormats()
{sfIssuer, soeREQUIRED},
{sfCredentialType, soeREQUIRED},
});

add(sfPermission.jsonName.c_str(),
sfPermission.getCode(),
{{sfPermissionValue, soeREQUIRED}});
}

InnerObjectFormats const&
Expand Down
97 changes: 97 additions & 0 deletions src/libxrpl/protocol/Permissions.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2024 Ripple Labs Inc.

Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.

THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================

#include <xrpl/protocol/Permissions.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/SOTemplate.h>
#include <xrpl/protocol/jss.h>

namespace ripple {

Permission::Permission()

Check warning on line 27 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L27

Added line #L27 was not covered by tests
{
granularPermissionMap = {
{"TrustlineAuthorize", gpTrustlineAuthorize},
{"TrustlineFreeze", gpTrustlineFreeze},
{"TrustlineUnfreeze", gpTrustlineUnfreeze},
{"AccountDomainSet", gpAccountDomainSet},
{"AccountEmailHashSet", gpAccountEmailHashSet},
{"AccountMessageKeySet", gpAccountMessageKeySet},
{"AccountTransferRateSet", gpAccountTransferRateSet},
{"AccountTickSizeSet", gpAccountTickSizeSet},
{"PaymentMint", gpPaymentMint},
{"PaymentBurn", gpPaymentBurn},
{"MPTokenIssuanceLock", gpMPTokenIssuanceLock},
{"MPTokenIssuanceUnlock", gpMPTokenIssuanceUnlock}};

granularTxTypeMap = {

Check warning on line 43 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L43

Added line #L43 was not covered by tests
{gpTrustlineAuthorize, ttTRUST_SET},
{gpTrustlineFreeze, ttTRUST_SET},
{gpTrustlineUnfreeze, ttTRUST_SET},
{gpAccountDomainSet, ttACCOUNT_SET},
{gpAccountEmailHashSet, ttACCOUNT_SET},
{gpAccountMessageKeySet, ttACCOUNT_SET},
{gpAccountTransferRateSet, ttACCOUNT_SET},
{gpAccountTickSizeSet, ttACCOUNT_SET},
{gpPaymentMint, ttPAYMENT},
{gpPaymentBurn, ttPAYMENT},
{gpMPTokenIssuanceLock, ttMPTOKEN_ISSUANCE_SET},
{gpMPTokenIssuanceUnlock, ttMPTOKEN_ISSUANCE_SET}};
}

Permission const&
Permission::getInstance()

Check warning on line 59 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L59

Added line #L59 was not covered by tests
{
static Permission const instance;
return instance;

Check warning on line 62 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L62

Added line #L62 was not covered by tests
}

std::optional<std::uint32_t>
Permission::getGranularValue(std::string const& name) const

Check warning on line 66 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L66

Added line #L66 was not covered by tests
{
auto const it = granularPermissionMap.find(name);
if (it != granularPermissionMap.end())
return static_cast<uint32_t>(it->second);

Check warning on line 70 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L70

Added line #L70 was not covered by tests

return std::nullopt;

Check warning on line 72 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L72

Added line #L72 was not covered by tests
}

std::optional<TxType>
Permission::getGranularTxType(GranularPermissionType const& gpType) const

Check warning on line 76 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L76

Added line #L76 was not covered by tests
{
auto const it = granularTxTypeMap.find(gpType);
if (it != granularTxTypeMap.end())
return it->second;

Check warning on line 80 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L80

Added line #L80 was not covered by tests

return std::nullopt;

Check warning on line 82 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L82

Added line #L82 was not covered by tests
}

bool
Permission::isProhibited(std::string const& name) const

Check warning on line 86 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L86

Added line #L86 was not covered by tests
{
// We do not allow delegating the following transaction permissions to other
// accounts for security reason.
if (name == "AccountSet" || name == "SetRegularKey" ||
name == "SignerListSet")
return true;

Check warning on line 92 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L91-L92

Added lines #L91 - L92 were not covered by tests

return false;

Check warning on line 94 in src/libxrpl/protocol/Permissions.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/Permissions.cpp#L94

Added line #L94 was not covered by tests
}

} // namespace ripple
43 changes: 39 additions & 4 deletions src/libxrpl/protocol/STParsedJSON.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <xrpl/beast/core/LexicalCast.h>
#include <xrpl/protocol/ErrorCodes.h>
#include <xrpl/protocol/LedgerFormats.h>
#include <xrpl/protocol/Permissions.h>
#include <xrpl/protocol/SField.h>
#include <xrpl/protocol/STAccount.h>
#include <xrpl/protocol/STAmount.h>
Expand Down Expand Up @@ -360,10 +361,44 @@
{
if (value.isString())
{
ret = detail::make_stvar<STUInt32>(
field,
beast::lexicalCastThrow<std::uint32_t>(
value.asString()));
if (field == sfPermissionValue)
{
std::string const strValue = value.asString();
auto const granularPermission =
Permission::getInstance().getGranularValue(
strValue);
if (!granularPermission)
{
// if it's not granular permission, parse as
// transaction type permission.
if (Permission::getInstance().isProhibited(
strValue))
{
// we do not allow delegating some transaction
// type permissions to other accounts for
// security reason.
error = invalid_data(json_name, fieldName);
return ret;
}
else
ret = detail::make_stvar<STUInt32>(

Check warning on line 384 in src/libxrpl/protocol/STParsedJSON.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/STParsedJSON.cpp#L384

Added line #L384 was not covered by tests
field,
static_cast<std::uint32_t>(

Check warning on line 386 in src/libxrpl/protocol/STParsedJSON.cpp

View check run for this annotation

Codecov / codecov/patch

src/libxrpl/protocol/STParsedJSON.cpp#L386

Added line #L386 was not covered by tests
TxFormats::getInstance().findTypeByName(
strValue) +
1));
}
else
ret = detail::make_stvar<STUInt32>(
field, *granularPermission);
}
else
{
ret = detail::make_stvar<STUInt32>(
field,
beast::lexicalCastThrow<std::uint32_t>(
value.asString()));
}
}
else if (value.isInt())
{
Expand Down
Loading
Loading