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 6 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
92 changes: 92 additions & 0 deletions include/xrpl/protocol/Permissions.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
//------------------------------------------------------------------------------
/*
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 {
TrustlineAuthorize = 65537,

TrustlineFreeze = 65538,

TrustlineUnfreeze = 65539,

AccountDomainSet = 65540,

AccountEmailHashSet = 65541,

AccountMessageKeySet = 65542,

AccountTransferRateSet = 65543,

AccountTickSizeSet = 65544,

PaymentMint = 65545,

PaymentBurn = 65546,

MPTokenIssuanceLock = 65547,

MPTokenIssuanceUnlock = 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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just added

{
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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

need to access private members in the functions, for example, granularPermissionMap and granularTxTypeMap

Copy link
Collaborator

Choose a reason for hiding this comment

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

They should be also static. There should be only one instance of those maps.

getInstance();

Permission(const Permission&) = delete;
Permission&
operator=(const Permission&) = delete;

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
3 changes: 3 additions & 0 deletions include/xrpl/protocol/STTx.h
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ class STTx final : public STObject, public CountedObject<STTx>
char status,
std::string const& escapedMetaData) const;

AccountID
getEffectiveAccountID() const;

private:
Expected<void, std::string>
checkSingleSign(RequireFullyCanonicalSig requireCanonicalSig) const;
Expand Down
228 changes: 228 additions & 0 deletions include/xrpl/protocol/STTxDelegated.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,228 @@
//------------------------------------------------------------------------------
/*
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_STTX_WRAPPER_H_INCLUDED
#define RIPPLE_PROTOCOL_STTX_WRAPPER_H_INCLUDED

#include <xrpl/protocol/STAccount.h>
#include <xrpl/protocol/STTx.h>
#include <functional>

namespace ripple {

// This class is a wrapper to deal with delegating in AccountPermission
// amendment. It wraps STTx, and delegates to STTx methods. The key change is
// getAccountID and operator[]. We need to first check if the transaction is
// delegated by another account by checking if the sfOnBehalfOf field is
// present. If it is present, we need to return the sfOnBehalfOf field as the
// account when calling getAccountID(sfAccount) and tx[sfAccount].
class STTxDelegated
{
private:
const STTx& tx_; // Wrap an instance of STTx
bool isDelegated_; // if the transaction is delegated by another account

public:
explicit STTxDelegated(STTx const& tx, bool isDelegated)
: tx_(tx), isDelegated_(isDelegated)
{
}

const STTx&
getSTTx() const
{
return tx_;
}

bool
isDelegated() const
{
return isDelegated_;
}

AccountID
getAccountID(SField const& field) const
{
if (field == sfAccount)
return tx_.isFieldPresent(sfOnBehalfOf) ? *tx_[~sfOnBehalfOf]
: tx_[sfAccount];
return tx_.getAccountID(field);
}

// template <class T>
// typename std::enable_if<
// !std::is_same<TypedField<T>, SF_ACCOUNT>::value,
// typename T::value_type>::type
// operator[](TypedField<T> const& f) const
// {
// return tx_[f];
// }

template <class T>
requires(!std::is_same<TypedField<T>, SF_ACCOUNT>::value)
typename T::value_type
operator[](TypedField<T> const& f) const
{
return tx_[f];
}

// When Type is SF_ACCOUNT and also field name is sfAccount, we need to
// check if the transaction is delegated by another account. If it is,
// return sfOnBehalfOf field instead.
template <class T>
requires std::is_same<TypedField<T>, SF_ACCOUNT>::value
AccountID
operator[](TypedField<T> const& f) const
{
if (f == sfAccount)
return tx_.isFieldPresent(sfOnBehalfOf) ? *tx_[~sfOnBehalfOf]
: tx_[sfAccount];
return tx_[f];
}

// template <class T>
// typename std::enable_if<
// std::is_same<TypedField<T>, SF_ACCOUNT>::value,
// AccountID>::type
// operator[](TypedField<T> const& f) const
// {
// if (f == sfAccount)
// return tx_.isFieldPresent(sfOnBehalfOf) ? *tx_[~sfOnBehalfOf]
// : tx_[sfAccount];
// return tx_[f];

template <class T>
std::optional<std::decay_t<typename T::value_type>>
operator[](OptionaledField<T> const& of) const
{
return tx_[of];
}

uint256
getTransactionID() const
{
return tx_.getTransactionID();
}

TxType
getTxnType() const
{
return tx_.getTxnType();
}

std::uint32_t
getFlags() const
{
return tx_.getFlags();
}

bool
isFieldPresent(SField const& field) const
{
return tx_.isFieldPresent(field);
}

Json::Value
getJson(JsonOptions options) const
{
return tx_.getJson(options);
}

void
add(Serializer& s) const
{
tx_.add(s);
}

unsigned char
getFieldU8(SField const& field) const
{
return tx_.getFieldU8(field);
}

std::uint32_t
getFieldU32(SField const& field) const
{
return tx_.getFieldU32(field);
}

uint256
getFieldH256(SField const& field) const
{
return tx_.getFieldH256(field);
}

Blob
getFieldVL(SField const& field) const
{
return tx_.getFieldVL(field);
}

STAmount const&
getFieldAmount(SField const& field) const
{
return tx_.getFieldAmount(field);
}

STPathSet const&
getFieldPathSet(SField const& field) const
{
return tx_.getFieldPathSet(field);
}

const STVector256&
getFieldV256(SField const& field) const
{
return tx_.getFieldV256(field);
}

const STArray&
getFieldArray(SField const& field) const
{
return tx_.getFieldArray(field);
}

Blob
getSigningPubKey() const
{
return tx_.getSigningPubKey();
}

Blob
getSignature() const
{
return tx_.getSignature();
}

bool
isFlag(std::uint32_t f) const
{
return tx_.isFlag(f);
}

SeqProxy
getSeqProxy() const
{
return tx_.getSeqProxy();
}
};

} // 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is an on-ledger object, not the transaction. It's empty if there is no permissions so it could be optional. Can save a bit of space this way, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes. make sense.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gregtatcam when there's no permissions, can we erase the SLE? so that we can still keep sfPermissions as required?

Copy link
Collaborator

@gregtatcam gregtatcam Jan 7, 2025

Choose a reason for hiding this comment

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

Do you mean delete SLE? I don't think we should delete it if there is no permissions. There is a transaction for this. Do you see a problem with making it optional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it's easier to just delete the ledger entry if there's no permission between two accounts. Is there any reason why should we keep it and just leave out the permissions field?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think there is a precedent for this, is there? A transaction is used to delete a ledger entry.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think there is a hard requirement that a specific txn is needed to delete an object. Ex. unfunded offers can be cancelled implicitly, associated NFT offers are automatically deleted when the NFT itself is deleted (see #4346)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can delete it then.

{sfOwnerNode, soeREQUIRED},
{sfPreviousTxnID, soeREQUIRED},
{sfPreviousTxnLgrSeq, soeREQUIRED},
}))
Loading