[GraphQL][DotMove][2/n] Introduce typeByName
#18797
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
manolisliolios
requested review from
amnn,
wlmyng,
stefan-mysten,
emmazzz and
suiwombat
as code owners
There was a problem hiding this comment.
Really good start on this one as well @manolisliolios, thank you! Themes on this PR:
- We may be able to simplify things by exposing a
multi_queryAPI forNamedMovePackage(also maybe not because it probably delegates to other data loaders). NamedType::queryshould return aTypeTag, and it should produce an error if we end up missing out a named address binding, so we can craft a legible error in that case.
|
Thanks a lot @amnn!
That was my concern and why I didn't try to jump towards that route fwiw (It uses one to two dataloaders internally, one for the external resolution (optional, only on external), and the package loader. |
manolisliolios
force-pushed
the
ml/dot-move-2-n
branch
from
25128c4
to
ea3731f
Compare
There was a problem hiding this comment.
Nice! Main comment here pertains to parsing error messages for TypeTag, and whether they are affected by this substitution with 0x0 -- everything else is nits.
I'm not certain that TypeTag's parsing errors mention the offsets that it found the problematic tokens at, so do check that. If it does not, then I don't think it's urgent to make the change I suggested (making the substituted address as long as the name it's replacing), but otherwise we should.
manolisliolios
force-pushed
the
ml/dot-move-1-n
branch
3 times, most recently
from
3331aee
to
7e3e6f2
Compare
manolisliolios
force-pushed
the
ml/dot-move-2-n
branch
from
ea3731f
to
1d62adc
Compare
manolisliolios
force-pushed
the
ml/dot-move-2-n
branch
from
1d62adc
to
bf2b163
Compare
manolisliolios
force-pushed
the
ml/dot-move-2-n
branch
from
bf2b163
to
6e4390a
Compare
## Description Introduces the external resolution, which requests "names" from a different graphql node. In reality, that'll be a mainnet RPC endpoint being used. ## Test plan e2e tests updated to start a secondary service that utilizes external resolution, and depend on that: ``` cargo nextest run --package sui-graphql-rpc --test dot_move_e2e --features pg_integration ``` ## Stack - #18797 - #18774 - #18770 --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: - [ ] REST API:
Description
Introduces
typeByNamethat resolves a given type using the equivalent dot move name.Test plan
You can run
cargo testfor unit tests (parsing of types).For e2e tests:
Stack
DotMoveresolution #18774Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.