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

Add NFT owner to API Server response #1858

Merged
merged 3 commits into from
Jan 10, 2025
Merged

Add NFT owner to API Server response #1858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d42c687
Add NFT owner to API Server response
OBorce Dec 24, 2024
1eb5cf8
fix additional tx data for CreateOrder in API Server
OBorce Dec 26, 2024
267343e
Add nonce in API server orders endpoints
OBorce Jan 8, 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
54 changes: 44 additions & 10 deletions api-server/api-server-common/src/storage/impls/in_memory/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ pub mod transactional;
use crate::storage::storage_api::{
block_aux_data::{BlockAuxData, BlockWithExtraData},
ApiServerStorageError, BlockInfo, CoinOrTokenStatistic, Delegation, FungibleTokenData,
LockedUtxo, Order, PoolBlockStats, TransactionInfo, Utxo, UtxoLock, UtxoWithExtraInfo,
LockedUtxo, NftWithOwner, Order, PoolBlockStats, TransactionInfo, Utxo, UtxoLock,
UtxoWithExtraInfo,
};
use common::{
address::Address,
chain::{
block::timestamp::BlockTimestamp,
tokens::{NftIssuance, TokenId},
Expand Down Expand Up @@ -55,7 +57,7 @@ struct ApiServerInMemoryStorage {
locked_utxo_table: BTreeMap<UtxoOutPoint, BTreeMap<BlockHeight, LockedUtxo>>,
address_locked_utxos: BTreeMap<String, BTreeSet<UtxoOutPoint>>,
fungible_token_issuances: BTreeMap<TokenId, BTreeMap<BlockHeight, FungibleTokenData>>,
nft_token_issuances: BTreeMap<TokenId, BTreeMap<BlockHeight, NftIssuance>>,
nft_token_issuances: BTreeMap<TokenId, BTreeMap<BlockHeight, NftWithOwner>>,
statistics:
BTreeMap<CoinOrTokenStatistic, BTreeMap<CoinOrTokenId, BTreeMap<BlockHeight, Amount>>>,
orders_table: BTreeMap<OrderId, BTreeMap<BlockHeight, Order>>,
Expand Down Expand Up @@ -599,7 +601,7 @@ impl ApiServerInMemoryStorage {
fn get_nft_token_issuance(
&self,
token_id: TokenId,
) -> Result<Option<NftIssuance>, ApiServerStorageError> {
) -> Result<Option<NftWithOwner>, ApiServerStorageError> {
Ok(self
.nft_token_issuances
.get(&token_id)
Expand Down Expand Up @@ -641,7 +643,7 @@ impl ApiServerInMemoryStorage {
(value.values().last().expect("not empty").token_ticker == ticker).then_some(key)
})
.chain(self.nft_token_issuances.iter().filter_map(|(key, value)| {
let value_ticker = match &value.values().last().expect("not empty") {
let value_ticker = match &value.values().last().expect("not empty").nft {
NftIssuance::V0(data) => data.metadata.ticker(),
};
(value_ticker == ticker).then_some(key)
Expand Down Expand Up @@ -803,7 +805,7 @@ impl ApiServerInMemoryStorage {

fn set_address_balance_at_height(
&mut self,
address: &str,
address: &Address<Destination>,
amount: Amount,
coin_or_token_id: CoinOrTokenId,
block_height: BlockHeight,
Expand All @@ -815,12 +817,38 @@ impl ApiServerInMemoryStorage {
.and_modify(|e| *e = amount)
.or_insert(amount);

self.update_nft_owner(coin_or_token_id, amount, address, block_height);

Ok(())
}

// The NFT owner is updated in both cases when it is spent as an input and transfered or
// created as an output. When the amount is 0 we set the owner to None as in the case of a Burn
fn update_nft_owner(
&mut self,
coin_or_token_id: CoinOrTokenId,
amount: Amount,
address: &Address<Destination>,
block_height: BlockHeight,
) {
let CoinOrTokenId::TokenId(token_id) = coin_or_token_id else {
return;
};

if let Some(by_height) = self.nft_token_issuances.get_mut(&token_id) {
let last = by_height.values().last().expect("not empty");
let owner = (amount > Amount::ZERO).then_some(address.as_object().clone());
let new = NftWithOwner {
nft: last.nft.clone(),
owner,
};
by_height.insert(block_height, new);
};
}
Comment on lines +838 to +847
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Let's add an assertion that the amount is either 0 or 1?
  2. You probably shouldn't update the map if the amount is zero. Currently this only works because update_tables_from_transaction_inputs is called before update_tables_from_transaction_outputs (as I can see), IMO it's not good for this function to depend on this.
  3. In update_tables_from_transaction_outputs, in the TxOutput::IssueNft case, set_nft_token_issuance is called after increase_address_amount. How is it supposed to work?

Copy link
Contributor Author

@OBorce OBorce Jan 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

  1. we are updating in both input (decrease_amount, i.e. amount 0 case) and output (increase_amount) as a nft utxo can be used as an input but not transferred anywhere or burned, in that case we set the owner to None.
  2. as the NFT id will not be in the database the update owner will do nothing. If the order is for some reason reversed the on conflict will just update the owner to the same owner.

Copy link
Contributor

Choose a reason for hiding this comment

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

2. we are updating in both input (decrease_amount, i.e. amount 0 case) and output (increase_amount) as a nft utxo can be used as an input but not transferred anywhere or burned, in that case we set the owner to None.

I see. A comment would be nice though.

But if calls to update_tables_from_transaction_outputs and update_tables_from_transaction_inputs are swapped, things would break, right? Asking this because I've just tried swapping them and all tests passed. Probably some tests are missing?

3. as the NFT id will not be in the database the update owner will do nothing. If the order is for some reason reversed the on conflict will just update the owner to the same owner.

Yeah I missed that, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we swap updating first the outputs then the inputs it will break only in a specific case where someone sends it from the same address to the same address. in that case the last decrease will set the owner to None. Will add a test for that just in case anyone swaps the processing order in the future.


fn set_address_locked_balance_at_height(
&mut self,
address: &str,
address: &Address<Destination>,
amount: Amount,
coin_or_token_id: CoinOrTokenId,
block_height: BlockHeight,
Expand All @@ -832,6 +860,8 @@ impl ApiServerInMemoryStorage {
.and_modify(|e| *e = amount)
.or_insert(amount);

self.update_nft_owner(coin_or_token_id, amount, address, block_height);

Ok(())
}

Expand Down Expand Up @@ -1060,11 +1090,15 @@ impl ApiServerInMemoryStorage {
token_id: TokenId,
block_height: BlockHeight,
issuance: NftIssuance,
owner: &Destination,
) -> Result<(), ApiServerStorageError> {
self.nft_token_issuances
.entry(token_id)
.or_default()
.insert(block_height, issuance);
self.nft_token_issuances.entry(token_id).or_default().insert(
block_height,
NftWithOwner {
nft: issuance,
owner: Some(owner.clone()),
},
);
Ok(())
}

Expand Down
11 changes: 5 additions & 6 deletions api-server/api-server-common/src/storage/impls/in_memory/transactional/read.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@ use std::collections::BTreeMap;

use common::{
chain::{
block::timestamp::BlockTimestamp,
tokens::{NftIssuance, TokenId},
Block, DelegationId, Destination, OrderId, PoolId, Transaction, UtxoOutPoint,
block::timestamp::BlockTimestamp, tokens::TokenId, Block, DelegationId, Destination,
OrderId, PoolId, Transaction, UtxoOutPoint,
},
primitives::{Amount, BlockHeight, CoinOrTokenId, Id},
};
use pos_accounting::PoolData;

use crate::storage::storage_api::{
block_aux_data::BlockAuxData, ApiServerStorageError, ApiServerStorageRead, BlockInfo,
CoinOrTokenStatistic, Delegation, FungibleTokenData, Order, PoolBlockStats, TransactionInfo,
Utxo, UtxoWithExtraInfo,
CoinOrTokenStatistic, Delegation, FungibleTokenData, NftWithOwner, Order, PoolBlockStats,
TransactionInfo, Utxo, UtxoWithExtraInfo,
};

use super::ApiServerInMemoryStorageTransactionalRo;
Expand Down Expand Up @@ -217,7 +216,7 @@ impl ApiServerStorageRead for ApiServerInMemoryStorageTransactionalRo<'_> {
async fn get_nft_token_issuance(
&self,
token_id: TokenId,
) -> Result<Option<NftIssuance>, ApiServerStorageError> {
) -> Result<Option<NftWithOwner>, ApiServerStorageError> {
self.transaction.get_nft_token_issuance(token_id)
}

Expand Down
14 changes: 8 additions & 6 deletions api-server/api-server-common/src/storage/impls/in_memory/transactional/write.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use std::collections::{BTreeMap, BTreeSet};

use common::{
address::Address,
chain::{
block::timestamp::BlockTimestamp,
tokens::{NftIssuance, TokenId},
Expand All @@ -28,8 +29,8 @@ use pos_accounting::PoolData;
use crate::storage::storage_api::{
block_aux_data::{BlockAuxData, BlockWithExtraData},
ApiServerStorageError, ApiServerStorageRead, ApiServerStorageWrite, BlockInfo,
CoinOrTokenStatistic, Delegation, FungibleTokenData, LockedUtxo, Order, PoolBlockStats,
TransactionInfo, Utxo, UtxoWithExtraInfo,
CoinOrTokenStatistic, Delegation, FungibleTokenData, LockedUtxo, NftWithOwner, Order,
PoolBlockStats, TransactionInfo, Utxo, UtxoWithExtraInfo,
};

use super::ApiServerInMemoryStorageTransactionalRw;
Expand Down Expand Up @@ -66,7 +67,7 @@ impl ApiServerStorageWrite for ApiServerInMemoryStorageTransactionalRw<'_> {

async fn set_address_balance_at_height(
&mut self,
address: &str,
address: &Address<Destination>,
amount: Amount,
coin_or_token_id: CoinOrTokenId,
block_height: BlockHeight,
Expand All @@ -81,7 +82,7 @@ impl ApiServerStorageWrite for ApiServerInMemoryStorageTransactionalRw<'_> {

async fn set_address_locked_balance_at_height(
&mut self,
address: &str,
address: &Address<Destination>,
amount: Amount,
coin_or_token_id: CoinOrTokenId,
block_height: BlockHeight,
Expand Down Expand Up @@ -219,8 +220,9 @@ impl ApiServerStorageWrite for ApiServerInMemoryStorageTransactionalRw<'_> {
token_id: TokenId,
block_height: BlockHeight,
issuance: NftIssuance,
owner: &Destination,
) -> Result<(), ApiServerStorageError> {
self.transaction.set_nft_token_issuance(token_id, block_height, issuance)
self.transaction.set_nft_token_issuance(token_id, block_height, issuance, owner)
}

async fn del_token_issuance_above_height(
Expand Down Expand Up @@ -456,7 +458,7 @@ impl ApiServerStorageRead for ApiServerInMemoryStorageTransactionalRw<'_> {
async fn get_nft_token_issuance(
&self,
token_id: TokenId,
) -> Result<Option<NftIssuance>, ApiServerStorageError> {
) -> Result<Option<NftWithOwner>, ApiServerStorageError> {
self.transaction.get_nft_token_issuance(token_id)
}

Expand Down
2 changes: 1 addition & 1 deletion api-server/api-server-common/src/storage/impls/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

pub const CURRENT_STORAGE_VERSION: u32 = 18;
pub const CURRENT_STORAGE_VERSION: u32 = 19;

pub mod in_memory;
pub mod postgres;
90 changes: 80 additions & 10 deletions api-server/api-server-common/src/storage/impls/postgres/queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use crate::storage::{
storage_api::{
block_aux_data::{BlockAuxData, BlockWithExtraData},
ApiServerStorageError, BlockInfo, CoinOrTokenStatistic, Delegation, FungibleTokenData,
LockedUtxo, Order, PoolBlockStats, TransactionInfo, Utxo, UtxoWithExtraInfo,
LockedUtxo, NftWithOwner, Order, PoolBlockStats, TransactionInfo, Utxo, UtxoWithExtraInfo,
},
};

Expand Down Expand Up @@ -240,7 +240,7 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {

pub async fn set_address_balance_at_height(
&mut self,
address: &str,
address: &Address<Destination>,
amount: Amount,
coin_or_token_id: CoinOrTokenId,
block_height: BlockHeight,
Expand All @@ -260,12 +260,62 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {
.await
.map_err(|e| ApiServerStorageError::LowLevelStorageError(e.to_string()))?;

let CoinOrTokenId::TokenId(token_id) = coin_or_token_id else {
return Ok(());
};

self.update_nft_owner(token_id, height, (amount > Amount::ZERO).then_some(address))
.await?;

Ok(())
}

async fn update_nft_owner(
&mut self,
token_id: TokenId,
height: i64,
owner: Option<&Address<Destination>>,
) -> Result<(), ApiServerStorageError> {
self.tx
.execute(
Comment on lines +267 to +280
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comments as for the other update_nft_owner.

r#"
WITH LastRow AS (
SELECT
nft_id,
block_height,
ticker,
issuance
FROM
ml.nft_issuance
WHERE
nft_id = $1
ORDER BY
block_height DESC
LIMIT 1
)
INSERT INTO ml.nft_issuance (nft_id, block_height, ticker, issuance, owner)
SELECT
lr.nft_id,
$2,
lr.ticker,
lr.issuance,
$3
FROM
LastRow lr
ON CONFLICT (nft_id, block_height) DO UPDATE
SET
ticker = EXCLUDED.ticker,
owner = EXCLUDED.owner;"#,
&[&token_id.encode(), &height, &owner.map(|o| o.as_object().encode())],
)
.await
.map_err(|e| ApiServerStorageError::LowLevelStorageError(e.to_string()))?;
Ok(())
}

pub async fn set_address_locked_balance_at_height(
&mut self,
address: &str,
address: &Address<Destination>,
amount: Amount,
coin_or_token_id: CoinOrTokenId,
block_height: BlockHeight,
Expand All @@ -285,6 +335,13 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {
.await
.map_err(|e| ApiServerStorageError::LowLevelStorageError(e.to_string()))?;

let CoinOrTokenId::TokenId(token_id) = coin_or_token_id else {
return Ok(());
};

self.update_nft_owner(token_id, height, (amount > Amount::ZERO).then_some(address))
.await?;

Ok(())
}

Expand Down Expand Up @@ -631,7 +688,8 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {
block_height bigint NOT NULL,
ticker bytea NOT NULL,
issuance bytea NOT NULL,
PRIMARY KEY (nft_id)
owner bytea,
PRIMARY KEY (nft_id, block_height)
);",
)
.await?;
Expand Down Expand Up @@ -2030,11 +2088,11 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {
pub async fn get_nft_token_issuance(
&self,
token_id: TokenId,
) -> Result<Option<NftIssuance>, ApiServerStorageError> {
) -> Result<Option<NftWithOwner>, ApiServerStorageError> {
let row = self
.tx
.query_opt(
"SELECT issuance FROM ml.nft_issuance WHERE nft_id = $1
"SELECT issuance, owner FROM ml.nft_issuance WHERE nft_id = $1
ORDER BY block_height DESC
LIMIT 1;",
&[&token_id.encode()],
Expand All @@ -2048,22 +2106,34 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {
};

let serialized_data: Vec<u8> = row.get(0);
let owner: Option<Vec<u8>> = row.get(1);

let issuance = NftIssuance::decode_all(&mut serialized_data.as_slice()).map_err(|e| {
let nft = NftIssuance::decode_all(&mut serialized_data.as_slice()).map_err(|e| {
ApiServerStorageError::DeserializationError(format!(
"Nft issuance data for nft id {} deserialization failed: {}",
token_id, e
))
})?;

Ok(Some(issuance))
let owner = owner
.map(|owner| {
Destination::decode_all(&mut owner.as_slice()).map_err(|e| {
ApiServerStorageError::DeserializationError(format!(
"Deserialization failed for nft owner {token_id}: {e}"
))
})
})
.transpose()?;

Ok(Some(NftWithOwner { nft, owner }))
}

pub async fn set_nft_token_issuance(
&mut self,
token_id: TokenId,
block_height: BlockHeight,
issuance: NftIssuance,
owner: &Destination,
) -> Result<(), ApiServerStorageError> {
let height = Self::block_height_to_postgres_friendly(block_height);

Expand All @@ -2073,8 +2143,8 @@ impl<'a, 'b> QueryFromConnection<'a, 'b> {

self.tx
.execute(
"INSERT INTO ml.nft_issuance (nft_id, block_height, issuance, ticker) VALUES ($1, $2, $3, $4);",
&[&token_id.encode(), &height, &issuance.encode(), ticker],
"INSERT INTO ml.nft_issuance (nft_id, block_height, issuance, ticker, owner) VALUES ($1, $2, $3, $4, $5);",
&[&token_id.encode(), &height, &issuance.encode(), ticker, &owner.encode()],
)
.await
.map_err(|e| ApiServerStorageError::LowLevelStorageError(e.to_string()))?;
Expand Down
Loading
Loading