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

Add nftoken_id, nftoken_ids and offer_id meta fields into NFT Tx responses #4447

Merged
merged 7 commits into from
May 18, 2023
Merged
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
3 changes: 3 additions & 0 deletions Builds/CMake/RippledCore.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,9 @@ target_sources (rippled PRIVATE
src/ripple/rpc/impl/ShardVerificationScheduler.cpp
src/ripple/rpc/impl/Status.cpp
src/ripple/rpc/impl/TransactionSign.cpp
src/ripple/rpc/impl/NFTokenID.cpp
src/ripple/rpc/impl/NFTokenOfferID.cpp
src/ripple/rpc/impl/NFTSyntheticSerializer.cpp
#[===============================[
main sources:
subdir: perflog
Expand Down
3 changes: 3 additions & 0 deletions src/ripple/protocol/jss.h
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,8 @@ JSS(nft_offer_index); // out nft_buy_offers, nft_sell_offers
JSS(nft_page); // in: LedgerEntry
JSS(nft_serial); // out: account_nfts
JSS(nft_taxon); // out: nft_info (clio)
JSS(nftoken_id); // out: insertNFTokenID
JSS(nftoken_ids); // out: insertNFTokenID
JSS(no_ripple); // out: AccountLines
JSS(no_ripple_peer); // out: AccountLines
JSS(node); // out: LedgerEntry
Expand All @@ -436,6 +438,7 @@ JSS(node_writes_delayed); // out::GetCounts
JSS(obligations); // out: GatewayBalances
JSS(offer); // in: LedgerEntry
JSS(offers); // out: NetworkOPs, AccountOffers, Subscribe
JSS(offer_id); // out: insertNFTokenOfferID
JSS(offline); // in: TransactionSign
JSS(offset); // in/out: AccountTxOld
JSS(open); // out: handlers/Ledger
Expand Down
58 changes: 58 additions & 0 deletions src/ripple/rpc/NFTSyntheticSerializer.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2023 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_RPC_NFTSYNTHETICSERIALIZER_H_INCLUDED
#define RIPPLE_RPC_NFTSYNTHETICSERIALIZER_H_INCLUDED
shawnxie999 marked this conversation as resolved.
Show resolved Hide resolved

#include <ripple/protocol/Protocol.h>
#include <ripple/protocol/STBase.h>

#include <functional>
#include <memory>

namespace Json {
class Value;
}

namespace ripple {

class TxMeta;
class STTx;

namespace RPC {

struct JsonContext;

/**
Adds common synthetic fields to transaction-related JSON responses

@{
Copy link
Contributor

Choose a reason for hiding this comment

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

is this @{ here on purpose?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like the intention is to provide Doxygen markdown. Perhaps this documents the intent: https://www.doxygen.nl/manual/commands.html#cmdaddtogroup

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this format is from the deliveredAmount file, so i just followed the same format

Copy link
Contributor

Choose a reason for hiding this comment

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

are we using doxygen markdown in this repo? if so are the generated docs hosted somewhere?

Copy link
Collaborator

Choose a reason for hiding this comment

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

There have been people who were excited about Doxygen years ago. Certainly any Doxygen generated from the current source code is going to be very spotty. My past experience with Doxygen is it takes a lot of dedication to make truly useful Doxygen documentation. I don't think that level of effort has ever gone in. It's certainly not happening right now.

Speaking only for myself I'd be fine if all of the Doxygen markdown were removed. But I don't object to it being present either.

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 leaving it wouldn't really matter.

*/
void
insertNFTSyntheticInJson(
Json::Value&,
RPC::JsonContext const&,
std::shared_ptr<STTx const> const&,
TxMeta const&);
/** @} */

} // namespace RPC
} // namespace ripple

#endif
68 changes: 68 additions & 0 deletions src/ripple/rpc/NFTokenID.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2023 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_RPC_NFTOKENID_H_INCLUDED
#define RIPPLE_RPC_NFTOKENID_H_INCLUDED
shawnxie999 marked this conversation as resolved.
Show resolved Hide resolved

#include <ripple/protocol/Protocol.h>

#include <functional>
#include <memory>

namespace Json {
class Value;
}

namespace ripple {

class TxMeta;
class STTx;

namespace RPC {

/**
Add a `nftoken_ids` field to the `meta` output parameter.
The field is only added to successful NFTokenMint, NFTokenAcceptOffer,
and NFTokenCancelOffer transactions.

Helper functions are not static because they can be used by Clio.
@{
Copy link
Contributor

Choose a reason for hiding this comment

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

is this @{ here on purpose?

*/
bool
canHaveNFTokenID(
std::shared_ptr<STTx const> const& serializedTx,
TxMeta const& transactionMeta);

std::optional<uint256>
getNFTokenIDFromPage(TxMeta const& transactionMeta);

std::vector<uint256>
getNFTokenIDFromDeletedOffer(TxMeta const& transactionMeta);
shawnxie999 marked this conversation as resolved.
Show resolved Hide resolved

void
insertNFTokenID(
Json::Value& response,
std::shared_ptr<STTx const> const& transaction,
TxMeta const& transactionMeta);
/** @} */

} // namespace RPC
} // namespace ripple

#endif
64 changes: 64 additions & 0 deletions src/ripple/rpc/NFTokenOfferID.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2023 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_RPC_NFTOKENOFFERID_H_INCLUDED
#define RIPPLE_RPC_NFTOKENOFFERID_H_INCLUDED
Copy link
Contributor

Choose a reason for hiding this comment

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

cc @scottschurr - is this the right directory for these files? it seems like this directory should contain implementations of RPC calls, not implementations of helpers, which is what these are to me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Regarding

is this the right directory for these files?

Good question, and something I was not thinking about during my review.

The best I can suggest is that we look at the placement of similar functionality. If we use the insertDeliveredAmount() function as an example, then we see that the header file is in src/ripple/rpc and the implementation is in src/ripple/rpc/impl. So, if we follow that example, then these files are in the right places.

On the other hand, if we use the RPCHelpers collection of utilities as our example, then we see that both the header and the implementation are in src/ripple/rpc/impl. That would suggest the new header files should be moved down one level to src/ripple/rpc/impl .

I don't personally have a strong opinion, since I don't play in this section of the code very often. I think it would be fine to leave these files where they are. But I'd also be fine if both the headers and implementation files were in impl. There may also be other viable ways of organizing the files.


#include <ripple/protocol/Protocol.h>

#include <functional>
#include <memory>

namespace Json {
class Value;
}

namespace ripple {

class TxMeta;
class STTx;

namespace RPC {

/**
Add an `offer_id` field to the `meta` output parameter.
The field is only added to successful NFTokenCreateOffer transactions.

Helper functions are not static because they can be used by Clio.
@{
*/
bool
canHaveNFTokenOfferID(
std::shared_ptr<STTx const> const& serializedTx,
TxMeta const& transactionMeta);

std::optional<uint256>
getOfferIDFromCreatedOffer(TxMeta const& transactionMeta);
shawnxie999 marked this conversation as resolved.
Show resolved Hide resolved

void
insertNFTokenOfferID(
Json::Value& response,
std::shared_ptr<STTx const> const& transaction,
TxMeta const& transactionMeta);
/** @} */

} // namespace RPC
} // namespace ripple

#endif
3 changes: 3 additions & 0 deletions src/ripple/rpc/handlers/AccountTx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include <ripple/resource/Fees.h>
#include <ripple/rpc/Context.h>
#include <ripple/rpc/DeliveredAmount.h>
#include <ripple/rpc/NFTSyntheticSerializer.h>
#include <ripple/rpc/Role.h>
#include <ripple/rpc/impl/RPCHelpers.h>

Expand Down Expand Up @@ -307,6 +308,8 @@ populateJsonResponse(
jvObj[jss::validated] = true;
insertDeliveredAmount(
jvObj[jss::meta], context, txn, *txnMeta);
insertNFTSyntheticInJson(
jvObj, context, txn->getSTransaction(), *txnMeta);
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/ripple/rpc/handlers/Tx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#include <ripple/rpc/Context.h>
#include <ripple/rpc/DeliveredAmount.h>
#include <ripple/rpc/GRPCHandlers.h>
#include <ripple/rpc/NFTSyntheticSerializer.h>
#include <ripple/rpc/impl/RPCHelpers.h>

namespace ripple {
Expand Down Expand Up @@ -295,6 +296,8 @@ populateJsonResponse(
response[jss::meta] = meta->getJson(JsonOptions::none);
insertDeliveredAmount(
response[jss::meta], context, result.txn, *meta);
insertNFTSyntheticInJson(
response, context, result.txn->getSTransaction(), *meta);
}
}
response[jss::validated] = result.validated;
Expand Down
50 changes: 50 additions & 0 deletions src/ripple/rpc/impl/NFTSyntheticSerializer.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2023 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 <ripple/rpc/NFTSyntheticSerializer.h>

#include <ripple/app/ledger/LedgerMaster.h>
#include <ripple/app/ledger/OpenLedger.h>
#include <ripple/app/misc/Transaction.h>
#include <ripple/ledger/View.h>
#include <ripple/net/RPCErr.h>
#include <ripple/protocol/AccountID.h>
#include <ripple/protocol/Feature.h>
#include <ripple/rpc/Context.h>
#include <ripple/rpc/NFTokenID.h>
#include <ripple/rpc/NFTokenOfferID.h>
#include <ripple/rpc/impl/RPCHelpers.h>
#include <boost/algorithm/string/case_conv.hpp>

namespace ripple {
namespace RPC {

void
insertNFTSyntheticInJson(
Json::Value& response,
RPC::JsonContext const& context,
std::shared_ptr<STTx const> const& transaction,
TxMeta const& transactionMeta)
{
insertNFTokenID(response[jss::meta], transaction, transactionMeta);
insertNFTokenOfferID(response[jss::meta], transaction, transactionMeta);
}

} // namespace RPC
} // namespace ripple
Loading