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

feat: add search api of graphql #1234

Merged
merged 11 commits into from
Dec 28, 2022

Conversation

Naupio
Copy link
Contributor

@Naupio Naupio commented Dec 22, 2022

What problem does this PR solve?

warning: this pr depend on #1227

issue: #1231
add search api of graphql
example
keyword can be: udt name| account eth_address | address | transaction hash | block number

query {
  search_keyword(input: { keyword: "UDT"}){
    type
    id
  }
}

Check List

Test

  • unit test

Task

  • none

@coveralls
Copy link
Collaborator

coveralls commented Dec 22, 2022

Pull Request Test Coverage Report for Build 48fb303facb49abd20ab27fc6e8bd39cfef58f65-PR-1234

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 12 of 15 (80.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.4%) to 38.674%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/godwoken_explorer/graphql/resolovers/account.ex 0 1 0.0%
lib/godwoken_explorer/graphql/resolovers/search.ex 12 14 85.71%
Totals Coverage Status
Change from base Build ffccc176c0fa74ac792df0b8ad38facc486fd920: 0.4%
Covered Lines: 2438
Relevant Lines: 6304

💛 - Coveralls

@FrederLu
Copy link

image

image

image

When using Bridged Token and ERC20/ERC721/ERC1155 names, it will prompt that no information can be found.

@Naupio
Copy link
Contributor Author

Naupio commented Dec 23, 2022

@Keith-CY @JeffreyMa597 we need discuss some feat of this PR.

previously, we have search_udt api for search udt with fuzzy_name that returning a pagination list resut paginate_search_udts object

in here, we add graphql api search_keyword to replace restful api of search. but the returning object is search_result witch with type and id field for redirect the next page.

there a problem of using search_keyword api with UDT name which have more than one record will returning not_found like this

  "errors": [
    {
      "locations": [
        {
          "column": 3,
          "line": 17
        }
      ],
      "message": "not_found",
      "path": [
        "search_keyword"
      ]
    }

cc @FrederLu

@Naupio
Copy link
Contributor Author

Naupio commented Dec 23, 2022

query example of testnet-stg env

query {
  search_udt(input: { fuzzy_name: "%ERC%", limit: 1 }) {
    entries {
      id
      name
      symbol
      type
      contract_address_hash
    }
    metadata {
      total_count
      before
      after
    }
  }

  search_keyword(
    input: { keyword: "%MSHKUUPS%" }
  ) {
    type
    id
  }
}
{
  "data": {
    "search_keyword": null,
    "search_udt": {
      "entries": [
        {
          "contract_address_hash": "0xfa5a662b13208070e665ac8239bff9ad6e78a020",
          "id": 92081,
          "name": "MSHK ERC721 UUPS Token",
          "symbol": "MSHKUUPS",
          "type": "NATIVE"
        }
      ],
      "metadata": {
        "after": "g3QAAAABZAACaWRiAAFnsQ==",
        "before": null,
        "total_count": 808
      }
    }
  },
  "errors": [
    {
      "locations": [
        {
          "column": 3,
          "line": 17
        }
      ],
      "message": "not_found",
      "path": [
        "search_keyword"
      ]
    }
  ]
}

@Keith-CY
Copy link
Member

@zmcNotafraid please have a review

fix: erc20 udt return no minted_count
@Keith-CY
Copy link
Member

As mentioned in #618 (comment), if the keyword is the address of an uninitialized account, Address but not Account will be returned in json API. What would it be like in the graphql API if the address is of an uninitialized account.

@FrederLu
Copy link

Is an uninitialized account an address that doesn't exist? If you use a non-existent address graphql prompts not found.
image

@Keith-CY
Copy link
Member

Is an uninitialized account an address that doesn't exist? If you use a non-existent address graphql prompts not found. image

TL;DR;
An uninitialized account is an account/address that doesn't exist on-chain. It would be null/not found if an uninitialized account appears in response but now info of an uninitialized account/address is expected to be returned within the response(most fields are empty except the .bit domain).


Before this feature, if an account hasn't sent a transaction, it will not be initialized on the chain and it will not be stored in the database of gwscan. So the API returns not found. This case leads to a potential issue that .bit domain cannot be returned along with some APIs. Let's say that we have a request for a transaction and the to is an initialized account. The response would be

from_account: From Account
from_address: 0x.....
to_account: null
to_address: 0x......

If to_address has a corresponding .bit domain, it cannot be returned within to_account because it's null

This feature changed the behavior of gwscan. Even if the account has not sent a transaction(only appears in to of a transaction), it will still be stored in the database of gwscan and the response of requesting the transaction above would be

from_account: From Account
from_address: 0x.....
to_account: To Account
to_address: 0x.....

Now to's .bit domain could be returned within To Account.

@Naupio
Copy link
Contributor Author

Naupio commented Dec 27, 2022

warning: change account api result to union type 'account_or_address'
link issue: #1235

@FrederLu @Keith-CY
example

    query {
      account(input: {address: "0x59b670e9fa9d0a427751af201d676719a970857b"}){
        ... on Account{
          type
          eth_address
        }
      }
    }
    query {
      account(input: {address: "0x59b670e9fa9d0a427751af201d676719a970857e"}){
        ... on Address{
          eth_address
        }
      }
    }

zmcNotafraid and others added 7 commits December 28, 2022 17:41
Signed-off-by: Miles Zhang <mingchang555@hotmail.com>
Signed-off-by: Miles Zhang <mingchang555@hotmail.com>
Signed-off-by: Miles Zhang <mingchang555@hotmail.com>
Signed-off-by: Miles Zhang <mingchang555@hotmail.com>
Signed-off-by: Miles Zhang <mingchang555@hotmail.com>
@Naupio Naupio reopened this Dec 28, 2022
@Naupio Naupio removed the request for review from Keith-CY December 28, 2022 09:43
@Naupio Naupio merged commit 9d51236 into Magickbase:godwoken-v1-testnet-prod Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants