Skip to content

Conversation

@ales-otf
Copy link
Contributor

@ales-otf ales-otf commented Jun 12, 2025

Description

Turns out NetUid type in metadata represented as a wrapper around u16. This PR fixes it by manual implementation of TypeInfo to make NetUid have the same type id as u16.

Related Issue(s)

  • Closes #[issue number]

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Other (please describe):

Breaking Change

If this PR introduces a breaking change, please provide a detailed description of the impact and the migration path for existing applications.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have run cargo fmt and cargo clippy to ensure my code is formatted and linted correctly
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Screenshots (if applicable)

Please include any relevant screenshots or GIFs that demonstrate the changes made.

Additional Notes

Please provide any additional information or context that may be helpful for reviewers.

@ales-otf ales-otf marked this pull request as ready for review June 12, 2025 17:53
@ales-otf ales-otf marked this pull request as draft June 12, 2025 17:54
@ales-otf ales-otf force-pushed the fix/netuid-compatibility branch from 876454e to 0d924bf Compare June 12, 2025 18:00
@ales-otf ales-otf changed the title Trigger CI Fix NetUid metadata issue Jun 12, 2025
@ales-otf ales-otf marked this pull request as ready for review June 12, 2025 19:06
@ales-otf ales-otf marked this pull request as draft June 12, 2025 19:11
@ales-otf ales-otf force-pushed the fix/netuid-compatibility branch from 38ea027 to 16488f4 Compare June 12, 2025 19:22
@ales-otf ales-otf marked this pull request as ready for review June 12, 2025 19:23
sam0x17
sam0x17 previously approved these changes Jun 12, 2025
@sam0x17 sam0x17 added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Jun 12, 2025
@sam0x17
Copy link
Contributor

sam0x17 commented Jun 12, 2025

I'm gonna merge it, the failures are all related to the current network outages going on in public clouds

@thewhaleking
Copy link
Contributor

I'm gonna merge it, the failures are all related to the current network outages going on in public clouds

I don't believe the e2e failures are related to network outages:

            result = bytes(
>               encode_by_type_string(type_string, self.runtime.registry, value)
            )
E           ValueError: Invalid type for data: 1 of type <class 'int'>, type_def: Composite(TypeDefComposite { fields: [Field { name: None, ty: UntrackedSymbol { id: 40, marker: PhantomData<fn() -> core::any::TypeId> }, type_name: Some("u16"), docs: [] }] })

.venv/lib/python3.12/site-packages/async_substrate_interface/types.py:906: ValueError
=========================== short test summary info ============================
FAILED tests/e2e_tests/test_metagraph.py::test_metagraph - ValueError: Invalid type for data: 2 of type <class 'int'>, type_def: Composite(TypeDefComposite { fields: [Field { name: None, ty: UntrackedSymbol { id: 40, marker: PhantomData<fn() -> core::any::TypeId> }, type_name: Some("u16"), docs: [] }] })
FAILED tests/e2e_tests/test_metagraph.py::test_metagraph_info - ValueError: Invalid type for data: 1 of type <class 'int'>, type_def: Composite(TypeDefComposite { fields: [Field { name: None, ty: UntrackedSymbol { id: 40, marker: PhantomData<fn() -> core::any::TypeId> }, type_name: Some("u16"), docs: [] }] })

These seem related to the same type issues as previously.

@ales-otf ales-otf marked this pull request as draft June 13, 2025 10:33
@ales-otf
Copy link
Contributor Author

converted to draft, to prevent merging

@ales-otf
Copy link
Contributor Author

ales-otf commented Jun 13, 2025

marker: P

I'm gonna merge it, the failures are all related to the current network outages going on in public clouds

I don't believe the e2e failures are related to network outages:

            result = bytes(
>               encode_by_type_string(type_string, self.runtime.registry, value)
            )
E           ValueError: Invalid type for data: 1 of type <class 'int'>, type_def: Composite(TypeDefComposite { fields: [Field { name: None, ty: UntrackedSymbol { id: 40, marker: PhantomData<fn() -> core::any::TypeId> }, type_name: Some("u16"), docs: [] }] })

.venv/lib/python3.12/site-packages/async_substrate_interface/types.py:906: ValueError
=========================== short test summary info ============================
FAILED tests/e2e_tests/test_metagraph.py::test_metagraph - ValueError: Invalid type for data: 2 of type <class 'int'>, type_def: Composite(TypeDefComposite { fields: [Field { name: None, ty: UntrackedSymbol { id: 40, marker: PhantomData<fn() -> core::any::TypeId> }, type_name: Some("u16"), docs: [] }] })
FAILED tests/e2e_tests/test_metagraph.py::test_metagraph_info - ValueError: Invalid type for data: 1 of type <class 'int'>, type_def: Composite(TypeDefComposite { fields: [Field { name: None, ty: UntrackedSymbol { id: 40, marker: PhantomData<fn() -> core::any::TypeId> }, type_name: Some("u16"), docs: [] }] })

These seem related to the same type issues as previously.

hmm this looks like some other type. Some(u16) is Option. Also, NetUid type id is 39 - not 40. ok, it tells it's u16, but what's the problem then... will check it

even more interesting, that with a locally running node all the tests passed. well, at least e2e from btcli, which failed here...

@ales-otf
Copy link
Contributor Author

ales-otf commented Jun 13, 2025

are we sure CI runs on this branch's node?

Status: Image is up to date for ghcr.io/opentensor/subtensor-localnet:devnet-ready
ghcr.io/opentensor/subtensor-localnet:devnet-ready


ok, it looks like it's -s flag when you run the tests. not familiar with python infra, but when you run something like:

BUILD_BINARY=1 USE_DOCKER=0 LOCALNET_SH_PATH=~/Desktop/subtensor/scripts/localnet.sh uv run pytest tests/e2e_tests

all tests pass. but if you add -s (which I thought is just for the log verbosity). the tests fail the same way they fail here


UPD

hmm no, local errors are not the same. local errors related to json decoding from color/rich output from my terminal. it looks like the tests rely on stdout parsing. figuring out how to workaround it. because it seems that the tests are running on a node without this fix.

for example, it tries to decode something like this, when I run it locally:

s = '\x1b[1m{\x1b[0m"success": true, "data": \x1b[1m{\x1b[0m"name": "new_wallet", "path": "/tmp/btcli-e2e-wallet-Bob", "ho...9VyE", "coldkey_ss58": "5DWpabhPnwaBDsttEm3DCHny6snZkXGfq11zygUSQjAmYKjb"\x1b[1m}\x1b[0m, "error": ""\x1b[1m}\x1b[0m\n'

NO_COLOR=1 env var turns of the colors, but still json looks like this

@basfroman basfroman marked this pull request as ready for review June 13, 2025 21:12
@sam0x17 sam0x17 merged commit e2675b4 into devnet-ready Jun 13, 2025
55 checks passed
@ales-otf ales-otf mentioned this pull request Jun 13, 2025
13 tasks
This was referenced Jun 18, 2025
@sam0x17 sam0x17 mentioned this pull request Jun 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants