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

GrantedNftAllowance does not match NftAllowance #191

Open
bugbytesinc opened this issue Apr 22, 2022 · 5 comments
Open

GrantedNftAllowance does not match NftAllowance #191

bugbytesinc opened this issue Apr 22, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@bugbytesinc
Copy link

Description

NftAllowance supports both "all nfts" and specific NFT allowances, but the information echoed back from the HAPI only indicates those allowances for "all nfts" in the GrantedNftAllowance response.

Steps to reproduce

message GrantedNftAllowance {

Additional context

No response

Hedera network

other

Version

current protobuf

Operating system

No response

@bugbytesinc bugbytesinc added the bug Something isn't working label Apr 22, 2022
@steven-sheehy
Copy link
Member

This is intentional. The number of NFT allowances is unbounded and queries can't support returning unbounded information. You can use mirror nodes to view the list of NFT allowances.

@aBFT4LIFE
Copy link

This is intentional. The number of NFT allowances is unbounded and queries can't support returning unbounded information. You can use mirror nodes to view the list of NFT allowances.

Hi There, can you please clarify with specific and exact information as to why this can't be supported. Are there limitations of the network that are being considered here? Does allowing unbounded information cause the network to become in any way unstable or throw a large amount of errors?

Context and detail is required here please asap.

@rbair23
Copy link
Member

rbair23 commented Apr 22, 2022

We are designing the network to support arbitrarily large numbers of NFT associations, allowances, NFTs, accounts, Smart Contracts, Smart Contract data, and so forth. We will scale as many entity types as possible beyond what you can do with RAM. For this to work, some groundbreaking work had to be done on virtualizing the merkle tree system onto the disk (what we call Virtual Merkle). The Hedera implementation of an on-disk merkle tree is much faster than the equivalent system on Ethereum and, to the best of our knowledge, is more efficient than any other such system in the world.

Instead of having to store all data in RAM, we can store all data on disk, and do so maintaining all our TPS guarantees, and that allows us to scale out far beyond what we could do in the past. We are truly designing for planet scale.

Given the scope of entities, associations, allowances, and other state that is available, we clearly would not want to support unbounded and unpaged information to a query. If a client made a query to a node and that required it to read 1M associations worth of data from disk and stream it all to the client, it could feasibly consume the disk I/O and network I/O on that node to a large extent. That could impact the integrity of the node which is clearly not something we would permit.

Typical networked systems will scale out (i.e. add more computers) to a cluster to increase the resources that can be supported by that application. When we scale out the number of computers on the consensus network, we are scaling out trust, but not resources. Each and every computer on that network holds the same state and handles the same transactions so adding computers doesn't increase resources available, which is why our main consensus nodes don't store historical data, they store the current state.

The mirror node networks however are more traditional. In a mirror node network, each computer added to the mirror node results in increased resources but does not scale the trust, which was established through the main consensus nodes. So we can scale out to support arbitrary numbers of requests per second on a mirror node and include all kinds of information including historical information.

For almost all types of queries, it is therefore better to query a mirror node instead of a consensus node, because it will scale better and leave more computation and I/O available on the consensus nodes for doing what they alone can do -- accept incoming transactions, come to consensus, execute transactions, verify results, and generate the record files (which are the equivalent of a block chain in our system).

In no circumstance do we have any queries which will affect the integrity of the network and we will never permit such a query to exist. Stability and security are our number one priorities and these are foremost in every design discussion we have. Where possible, we strongly recommend using mirror nodes for querying data, and consensus nodes for transacting data. It is just the most efficient and effective way to use these systems based on their natural strengths. The main consensus nodes will always need to be the place to get receipts so you know your transaction was accepted and handled, but for almost every other type of query you will be far better off going to a mirror node.

@bugbytesinc
Copy link
Author

There are a number of overreaching policy decisions/implied guidance this public implementation assumes. Could you lead us to the HIP or other public discussion regarding why this departure from the symmetric HAPI design that we have had up and to this point?

@rbair23
Copy link
Member

rbair23 commented Apr 25, 2022

@bugbytesinc from your comment I think the implied subtext is "this was a surprise to me and should have been discussed somewhere". I am sorry for any miscommunication.

In terms of previous HIPs that made the same argument, see https://github.com/hashgraph/hedera-improvement-proposal/blob/master/HIP/hip-367.md:

Two free queries, getAccountInfo and getAccountBalance, return token association data.

This HIP formally deprecates the token information returned by getAccountInfo and getAccountBalance. Within 6 months this information will no longer be retrieved from consensus nodes. Instead, clients must query mirror nodes for this information.

The challenge with these queries is that they require numerous uncharged map look-ups to find and retrieve the token information, and when token limits are removed, these queries may take an arbitrary amount of time to complete. For this reason, and to maintain compatibility during the deprecation period, we will continue to return at most 1000 (tokens.maxRelsPerInfoQuery) results. The order of these results is explicitly not specified. Thus, if you have 1001 token associations, we will only return 1,000 of them, and are free to return any 1,000. Instead, clients are strongly encouraged to query this information from mirror nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants