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.

Sign up for GitHub

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

XLS-46: DynamicNFT #5048

Merged
merged 47 commits into from
Jan 9, 2025
Merged

XLS-46: DynamicNFT #5048

Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
598a0e3
XLS-46 dNFTs PR first
Nov 27, 2023
d851678
dNFTs initial
xVet Nov 27, 2023
e30addc
added dnft amendment and placed guards
xVet Nov 28, 2023
0848a98
Merge branch 'XRPLF:develop' into dNFTs-XLS-46
xVet Nov 28, 2023
624e11e
added featureDNFT in Feature.h
xVet Nov 28, 2023
4cbde96
Merge branch 'dNFTs-XLS-46' of https://github.com/xVet/rippled into d…
xVet Nov 28, 2023
afaa675
Merge remote-tracking branch 'upstream/develop' into dNFTs-XLS-46
tequdev Feb 16, 2024
f6418aa
dNFTs
tequdev Feb 16, 2024
44790ab
rename dNFT to DynamicNFT
tequdev Feb 16, 2024
ef6ba44
clang-format
tequdev Feb 16, 2024
b9becc4
fix flag test
tequdev Feb 16, 2024
8fd988d
add standard license/disclaimer text
tequdev Feb 16, 2024
68a51e1
Merge remote-tracking branch 'upstream/develop' into dNFTs-XLS-46
tequdev May 31, 2024
750adda
chore
tequdev Jun 18, 2024
4b3480c
fix tests
tequdev Jun 18, 2024
f4ab8c6
Merge remote-tracking branch 'upstream/develop' into featureDynamicNFT
tequdev Jun 18, 2024
47106cc
Merge branch 'develop' into featureDynamicNFT
tequdev Jun 18, 2024
4ca5105
Merge branch 'develop' into featureDynamicNFT
tequdev Jun 21, 2024
579dc25
[FOLD] An incremental approach to the tfNFTokenMintMask variants
scottschurr Jun 27, 2024
cae9b62
[FOLD] Extend testNFTokenModify unit test
scottschurr Jun 27, 2024
23ecb99
[FOLD] Avoid NFTPage coalescing and splitting when URI changes
scottschurr Jun 27, 2024
2eb7559
fix sorting testcase
tequdev Jun 29, 2024
e9dd06c
Merge branch 'develop' into featureDynamicNFT
tequdev Jun 29, 2024
c89f41d
Merge branch 'develop' into featureDynamicNFT
tequdev Jul 3, 2024
1b2cf6d
Merge branch 'develop' into featureDynamicNFT
tequdev Jul 9, 2024
d52b659
Merge branch 'develop' into featureDynamicNFT
tequdev Jul 12, 2024
90aeef9
fix testcase name
tequdev Jul 12, 2024
4eea716
chore: fix typo
tequdev Jul 12, 2024
321459d
Merge branch 'develop' into featureDynamicNFT
tequdev Sep 23, 2024
b41b98a
Merge branch 'develop' into featureDynamicNFT
tequdev Sep 23, 2024
fc81bcc
clang-format
tequdev Sep 23, 2024
0f02668
Merge branch 'develop' into featureDynamicNFT
tequdev Sep 28, 2024
fa732bf
Merge branch 'develop' into featureDynamicNFT
tequdev Nov 1, 2024
16541e2
fix bad merge
tequdev Nov 1, 2024
a76809a
clang-format
tequdev Nov 1, 2024
e3f6542
Merge branch 'develop' into featureDynamicNFT
tequdev Nov 7, 2024
29e7d4d
fix merge
tequdev Nov 7, 2024
727ad8f
Merge branch 'develop' into featureDynamicNFT
tequdev Nov 12, 2024
d26ba5a
Merge branch 'develop' into featureDynamicNFT
tequdev Nov 13, 2024
a6aaa28
Merge branch 'develop' into featureDynamicNFT
tequdev Dec 10, 2024
7154534
Update src/xrpld/app/tx/detail/NFTokenModify.cpp
tequdev Dec 15, 2024
13156dd
Fix error handling that shoudn't be possible
tequdev Dec 15, 2024
f6f8084
clang-format
tequdev Dec 15, 2024
ca09114
Merge branch 'develop' into featureDynamicNFT
ximinez Dec 20, 2024
7390b1d
fix build error
tequdev Dec 22, 2024
f7311ba
fix tt tag
tequdev Dec 23, 2024
ccbed88
Merge branch 'develop' into featureDynamicNFT
tequdev Jan 9, 2025
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
3 changes: 2 additions & 1 deletion include/xrpl/protocol/Feature.h
View file
Open in desktop
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 = 77;
static constexpr std::size_t numFeatures = 78;

/** Amendments that this server supports and the default voting behavior.
Whether they are enabled depends on the Rules defined in the validated
Expand Down Expand Up @@ -370,6 +370,7 @@ extern uint256 const featureNFTokenMintOffer;
extern uint256 const fixReducedOffersV2;
extern uint256 const fixEnforceNFTokenTrustline;
extern uint256 const fixInnerObjTemplate2;
extern uint256 const featureDynamicNFT;

} // namespace ripple

Expand Down
37 changes: 23 additions & 14 deletions include/xrpl/protocol/TxFlags.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ constexpr std::uint32_t const tfBurnable = 0x00000001;
constexpr std::uint32_t const tfOnlyXRP = 0x00000002;
constexpr std::uint32_t const tfTrustLine = 0x00000004;
constexpr std::uint32_t const tfTransferable = 0x00000008;
constexpr std::uint32_t const tfMutable = 0x00000010;

// Prior to fixRemoveNFTokenAutoTrustLine, transfer of an NFToken between
// accounts allowed a TrustLine to be added to the issuer of that token
Expand All @@ -143,12 +144,19 @@ constexpr std::uint32_t const tfTransferable = 0x00000008;
// The fixRemoveNFTokenAutoTrustLine amendment disables minting with the
// tfTrustLine flag as a way to prevent the attack. But until the
// amendment passes we still need to keep the old behavior available.
constexpr std::uint32_t const tfNFTokenMintOldMask =
~(tfUniversal | tfBurnable | tfOnlyXRP | tfTrustLine | tfTransferable);

constexpr std::uint32_t const tfNFTokenMintMask =
~(tfUniversal | tfBurnable | tfOnlyXRP | tfTransferable);

constexpr std::uint32_t const tfNFTokenMintOldMask =
~( ~tfNFTokenMintMask | tfTrustLine);

// if featureDynamicNFT enabled then new flag allowing mutable URI available.
constexpr std::uint32_t const tfNFTokenMintOldMaskWithMutable =
~( ~tfNFTokenMintOldMask | tfMutable);

constexpr std::uint32_t const tfNFTokenMintMaskWithMutable =
~( ~tfNFTokenMintMask | tfMutable);

// NFTokenCreateOffer flags:
constexpr std::uint32_t const tfSellNFToken = 0x00000001;
constexpr std::uint32_t const tfNFTokenCreateOfferMask =
Expand All @@ -161,17 +169,17 @@ constexpr std::uint32_t const tfNFTokenCancelOfferMask = ~(tfUniversal);
constexpr std::uint32_t const tfNFTokenAcceptOfferMask = ~tfUniversal;

// Clawback flags:
constexpr std::uint32_t const tfClawbackMask = ~tfUniversal;
constexpr std::uint32_t const tfClawbackMask = ~tfUniversal;

// AMM Flags:
constexpr std::uint32_t tfLPToken = 0x00010000;
constexpr std::uint32_t tfWithdrawAll = 0x00020000;
constexpr std::uint32_t tfOneAssetWithdrawAll = 0x00040000;
constexpr std::uint32_t tfSingleAsset = 0x00080000;
constexpr std::uint32_t tfTwoAsset = 0x00100000;
constexpr std::uint32_t tfOneAssetLPToken = 0x00200000;
constexpr std::uint32_t tfLimitLPToken = 0x00400000;
constexpr std::uint32_t tfTwoAssetIfEmpty = 0x00800000;
constexpr std::uint32_t tfLPToken = 0x00010000;
constexpr std::uint32_t tfWithdrawAll = 0x00020000;
constexpr std::uint32_t tfOneAssetWithdrawAll = 0x00040000;
constexpr std::uint32_t tfSingleAsset = 0x00080000;
constexpr std::uint32_t tfTwoAsset = 0x00100000;
constexpr std::uint32_t tfOneAssetLPToken = 0x00200000;
constexpr std::uint32_t tfLimitLPToken = 0x00400000;
constexpr std::uint32_t tfTwoAssetIfEmpty = 0x00800000;
constexpr std::uint32_t tfWithdrawSubTx =
tfLPToken | tfSingleAsset | tfTwoAsset | tfOneAssetLPToken |
tfLimitLPToken | tfWithdrawAll | tfOneAssetWithdrawAll;
Expand All @@ -182,8 +190,9 @@ constexpr std::uint32_t tfWithdrawMask = ~(tfUniversal | tfWithdrawSubTx);
constexpr std::uint32_t tfDepositMask = ~(tfUniversal | tfDepositSubTx);

// BridgeModify flags:
constexpr std::uint32_t tfClearAccountCreateAmount = 0x00010000;
constexpr std::uint32_t tfBridgeModifyMask = ~(tfUniversal | tfClearAccountCreateAmount);
constexpr std::uint32_t tfClearAccountCreateAmount = 0x00010000;
constexpr std::uint32_t tfBridgeModifyMask =
~(tfUniversal | tfClearAccountCreateAmount);

// clang-format on

Expand Down
4 changes: 3 additions & 1 deletion include/xrpl/protocol/TxFormats.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,15 @@ enum TxType : std::uint16_t
/** This transaction type deletes a DID */
ttDID_DELETE = 50,


/** This transaction type creates an Oracle instance */
ttORACLE_SET = 51,

/** This transaction type deletes an Oracle instance */
ttORACLE_DELETE = 52,

/** This transaction type modify a NFToken */
ttNFTOKEN_MODIFY = 53,

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

For details, see: https://xrpl.org/amendments.html
Expand Down
1 change: 1 addition & 0 deletions include/xrpl/protocol/jss.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ JSS(NFTokenOffer); // ledger type.
JSS(NFTokenAcceptOffer); // transaction type.
JSS(NFTokenCancelOffer); // transaction type.
JSS(NFTokenCreateOffer); // transaction type.
JSS(NFTokenModify); // transaction type.
JSS(NFTokenPage); // ledger type.
JSS(LPTokenOut); // in: AMM Liquidity Provider deposit tokens
JSS(LPTokenIn); // in: AMM Liquidity Provider withdraw tokens
Expand Down
1 change: 1 addition & 0 deletions include/xrpl/protocol/nft.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ constexpr std::uint16_t const flagBurnable = 0x0001;
constexpr std::uint16_t const flagOnlyXRP = 0x0002;
constexpr std::uint16_t const flagCreateTrustLines = 0x0004;
constexpr std::uint16_t const flagTransferable = 0x0008;
constexpr std::uint16_t const flagMutable = 0x0010;

inline std::uint16_t
getFlags(uint256 const& id)
Expand Down
1 change: 1 addition & 0 deletions src/libxrpl/protocol/Feature.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ REGISTER_FEATURE(NFTokenMintOffer, Supported::yes, VoteBehavior::De
REGISTER_FIX (fixReducedOffersV2, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixEnforceNFTokenTrustline, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FIX (fixInnerObjTemplate2, Supported::yes, VoteBehavior::DefaultNo);
REGISTER_FEATURE(DynamicNFT, Supported::yes, VoteBehavior::DefaultNo);

// The following amendments are obsolete, but must remain supported
// because they could potentially get enabled.
Expand Down
9 changes: 9 additions & 0 deletions src/libxrpl/protocol/TxFormats.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,15 @@ TxFormats::TxFormats()
},
commonFields);

add(jss::NFTokenModify,
ttNFTOKEN_MODIFY,
{
{sfNFTokenID, soeREQUIRED},
{sfOwner, soeOPTIONAL},
{sfURI, soeOPTIONAL},
Copy link
Collaborator

Choose a reason for hiding this comment

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

The spec (XRPLF/XRPL-Standards#130 August 18, 2023 version) specifies that the sfURI field is required. Here it is listed as soeOPTIONAL. I think that soeOPTIONAL is a good choice, because that matches the behavior of NFTokenMint. Perhaps the spec needs to be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since you pointed it out, as of Jun 29 the sfURI field has been updated to an optional field.

Copy link
Collaborator

@shawnxie999 shawnxie999 Jul 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Is there any use case for removing the URI from an existing NFT? Is there any difference from burning the NFT compared to an NFT without a URI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Such a use case may not be common, but it is not impossible to say that there is no use case.

Since the NFTokenMint transaction can be executed with the URI field unspecified, it is reasonable that the NFTokenModify transaction can change the URI field to unspecified.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense. Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should there be a flag for removing the URI from the NFT, like with NFTokenMinter, instead of just using its absence?

},
commonFields);

add(jss::Clawback,
ttCLAWBACK,
{
Expand Down
Loading
Loading