Skip to content

feat: implement reading orb id on Android - #1382

Merged
pophilpo merged 3 commits into
aleksei/refactor-use-test-orb-idfrom
aleksei/fix-orb-id-on-android
Sep 15, 2026
Merged

pophilpo merged 3 commits into
aleksei/refactor-use-test-orb-idfrom
aleksei/fix-orb-id-on-android

Conversation

@alekseifedotov

@alekseifedotov alekseifedotov commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Read device id from /sys/devices/soc0/serial_number and save it as u32

Please use per-commit view for review

@alekseifedotov
alekseifedotov requested a review from a team as a code owner September 3, 2026 00:09
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
  • Blocking – Android build fails: orb_id_android.rs:27 combines Result<_, VarError> with Result<_, io::Error> via or_else; both async and blocking variants require the same error type. Convert the environment lookup explicitly and fall back with match.

  • Blocking – non-Linux/Android targets fail: lib.rs:2 renamed orb_id.rs without providing a module path for macOS or other targets, so Rust looks for the now-missing orb_id.rs.

  • Serde contract/test failure: orb_id_android.rs:12 derives externally tagged enum serialization ({"Mini":"abcd1234"}), while the existing contract and new test expect "abcd1234". Add serde(untagged) as in the Linux implementation.

@alekseifedotov alekseifedotov changed the title Aleksei/fix orb id on android fix: orb id on Android Sep 3, 2026
@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from 5701330 to 181ef30 Compare September 3, 2026 04:20
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Blocking issues

  • [P1] endpoints/src/lib.rs:19 — Removing the trailing / makes the next Url::join(suffix) replace the Orb ID segment. For example, .../orbs/ea2ea744 joined with status becomes .../orbs/status. Existing v1/v2 endpoint tests should fail. Keep format!("{orb_id}/").

  • [P1] orb-info/src/orb_id_android.rs:1-3 — Async-only imports are unconditional. Building orb-info on Android with default-features = false, features = ["orb-id"] fails because crate::from_file and the optional futures dependency are unavailable. Gate these imports with #[cfg(feature = "async")].

Base automatically changed from aleksei/add-android-build to main September 3, 2026 17:13
@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from 181ef30 to e898516 Compare September 3, 2026 19:31
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
  • [P0] endpoints/src/lib.rs:19 Preserve the trailing slash after orb_id. Without it, the second Url::join replaces the ID segment, producing paths such as /api/v2/orbs/status instead of /api/v2/orbs/<id>/status. Existing endpoint tests should fail.

  • [P1] orb-info/src/orb_id_android.rs:9 Deriving Serialize for the u32 newtype emits Android IDs as JSON numbers. Callers changed from &str to &OrbId will therefore send {"orbId":1234} instead of the established {"orbId":"1234"}, affecting attest and speed-test backend requests. Implement string serialization consistently with Linux.

  • [P1] orb-info/src/lib.rs:2 The original orb_id.rs was removed, but a replacement path is selected only for Linux and Android. Building orb-info with orb-id on macOS or other supported host targets now fails because Rust searches for the nonexistent orb_id.rs. Add a fallback or explicitly gate unsupported targets.

@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from e898516 to 04f787b Compare September 3, 2026 19:44
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
  • Blocker – endpoints/src/lib.rs:19: Removing the trailing / makes the following Url::join(suffix) replace the Orb-ID path segment. For example, .../orbs/ea2ea744 joined with status becomes .../orbs/status. Existing endpoint tests should fail. Preserve format!("{orb_id}/").

  • Blocker – orb-info/src/orb_id_android.rs:9: Android OrbId derives numeric serialization, but these APIs previously serialized IDs as JSON strings. Challenge, proof, and speed-test requests will send 1234 instead of "1234", likely breaking backend contracts. Serialize/deserialize it as a string while retaining numeric parsing internally.

  • Cross-platform regression – orb-info/src/lib.rs:2: orb_id.rs was renamed, but only Linux and Android receive replacement paths. Building orb-info with orb-id on macOS or other supported host platforms now fails because the default orb_id.rs no longer exists. Use the Linux implementation for all non-Android targets or retain a fallback module.

@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from 04f787b to d7befe4 Compare September 3, 2026 23:08
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
  • Blocker – endpoints/src/lib.rs:19: Removing the trailing / makes the subsequent join(suffix) replace the Orb ID path segment. For example, .../v1/1234 joined with auth becomes .../v1/auth, omitting the ID. Keep the trailing slash.

  • Blocker – orb-info/src/orb_id_android.rs:9: Derived Serialize encodes Android IDs as JSON numbers, while existing API requests send IDs as strings. Calls such as remote_api.rs:262 will therefore change from {"orbId":"1234"} to {"orbId":1234} on Android. Implement string serialization and test the request payloads.

  • Blocker – orb-info/src/lib.rs:2: On targets other than Linux or Android, neither path attribute applies, so Rust looks for the now-renamed orb_id.rs and compilation fails. Add an explicit fallback or restrict the module with an appropriate compile-time error.

@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from d7befe4 to a6c4ebe Compare September 3, 2026 23:18
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
  • orb-info/src/lib.rs:7: orb_id is now compiled even when the orb-id feature is disabled. On Linux this pulls in optional hex despite it not being enabled, breaking existing consumers such as ui that use orb-info with only orb-os-release. Add #[cfg(feature = "orb-id")] to the module declaration.

  • orb-info/src/orb_id_android.rs:1: Android builds with orb-id but without async fail because from_file and futures are imported unconditionally, while both dependencies are gated behind async. Gate those imports with #[cfg(feature = "async")].

@alekseifedotov
alekseifedotov changed the base branch from main to aleksei/refactor-use-test-orb-id September 4, 2026 00:01
@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch 2 times, most recently from 159d1a0 to 273f332 Compare September 4, 2026 00:05
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
  • Blocking — orb-info/src/orb_id_android.rs:32: String::deserialize requires serde::Deserialize in scope. Android builds enabling serde will fail to compile. Use <String as serde::Deserialize>::deserialize(deserializer) or import the trait.

@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from 273f332 to c9bc60f Compare September 4, 2026 00:12
@alekseifedotov alekseifedotov changed the title fix: orb id on Android feat: implement reading orb id on Android Sep 4, 2026
@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from c9bc60f to 14be670 Compare September 4, 2026 00:18
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
  • orb-info/src/orb_id_android.rs:52: Trim ORB_ID before parsing, as the Linux implementation does. Currently values such as "1234\n" fail on Android despite being accepted elsewhere; the sysfs fallback is already trimmed. The blocking path has the same issue at line 61.

@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from 14be670 to bf98856 Compare September 4, 2026 00:23
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

No concrete correctness, security, race-condition, or merge-blocking issues found in the PR changes.

@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from bf98856 to b62d1d7 Compare September 4, 2026 18:05
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
  • orb-info/src/orb_id_android.rs:53 and :62: ORB_ID is parsed without trimming, unlike the Linux implementation and file fallback. Values such as "1234\n" now fail with ParseIntError. Apply .trim() before parsing and test both read paths.

@alekseifedotov alekseifedotov added diffiulty:medium Hurt me plenty difficulty:easy Hey, not too rough and removed diffiulty:medium Hurt me plenty labels Sep 4, 2026
@alekseifedotov
alekseifedotov force-pushed the aleksei/fix-orb-id-on-android branch from b62d1d7 to 3c4a022 Compare September 9, 2026 00:29
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
  • [P2] Preserve non-Android host support — orb-info/src/lib.rs:1–10: The new compile_error! prevents macOS builds with orb-id enabled, including consumers such as orb-endpoints. Previously, ID parsing and environment-based reads worked there. Select the existing implementation for non-Android targets to avoid this regression.

Review covered all three commits. Tests were not run in the read-only environment.

@pophilpo
pophilpo force-pushed the aleksei/fix-orb-id-on-android branch from 3c4a022 to d66ee49 Compare September 15, 2026 17:42
@github-actions

Copy link
Copy Markdown
  • [P2] Preserve macOS support — orb-info/src/lib.rs:1–10: The new platform guard prevents previously portable consumers such as orb-endpoints from compiling or running native tests on macOS. Parsing IDs and using the ORB_ID override previously worked there without Orb hardware. Select the Android implementation specifically and retain the existing implementation for other supported hosts.

Tests could not run because the Rust toolchain attempted to write to the read-only filesystem.

@pophilpo
pophilpo merged commit 1dc6431 into main Sep 15, 2026
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

difficulty:easy Hey, not too rough

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants