Skip to content

Conversation

Copy link

Copilot AI commented Jan 20, 2026

Addresses review feedback from PR #32 to improve code quality and API ergonomics.

Spelling corrections:

  • Fixed typos in comments: returedreturned, therforetherefore, retieveretrieve

API improvement:

  • Changed TryInto<EthAddress> to implement for &Address instead of Address, preventing ownership transfer:
// Before: consumed the address
let eth_addr: EthAddress = cosmos_addr.try_into()?;  // cosmos_addr moved

// After: borrows the address
let eth_addr: EthAddress = (&cosmos_addr).try_into()?;  // cosmos_addr still usable

Test coverage:

  • Added test_get_txhash to verify SHA256 hash computation
  • Added test_trait_conversions for TryInto<EthAddress> and From<EthAddress> roundtrip
  • Added test_re_prefix to validate prefix changes preserve underlying bytes

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

jkilpatr and others added 5 commits January 20, 2026 15:00
These conversions are useful, but should maybe be explicit functions
rather than implicit conversions so that we can educate the user as to
how it's a bad idea in some situations.
This is easiser to use than the change prefix function in some contexts
By adding a txid to some error cases we can allow the caller to follow
the signed transaction even if the intial publishing call fails or times
out.
The one thing of note here is that althea_proto was rolled back because
everything in the 10 series is currently yanked.
Copy link
Author

Copilot AI commented Jan 20, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • gravitychain.io
    • Triggering command: /home/REDACTED/work/deep_space/deep_space/target/debug/deps/deep_space-f3ba4be92f2b6ea4 /home/REDACTED/work/deep_space/deep_space/target/debug/deps/deep_space-f3ba4be92f2b6ea4 -I depend/secp256k1/ -I depend/secp256k1/include -I depend/secp256k1/src -Wall -Wextra -Wno-unused-function -Wno-unused-parameter -DSECP256K1_API= ebug�� 7924265c4/out/fce6141bfa9bf74c-precomputed_ecmult.o -8f1e3becb5539c4-mtune=generic te-591f8e5905d6c-march=x86-64 c_macro2-5dfff39cc code_ident-4fec1-O0 lib/rustlib/x86_-ffunction-sections lib/rustlib/x86_-fdata-sections (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Add quality of life features and transaction ID handling Address PR review feedback: spelling fixes, test coverage, and non-consuming TryInto Jan 20, 2026
Copilot AI requested a review from jkilpatr January 20, 2026 21:16
@jkilpatr jkilpatr force-pushed the jkilpatr/txid-returns branch from b13bf44 to 083285b Compare January 20, 2026 22:11
@jkilpatr jkilpatr closed this Jan 20, 2026
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.

2 participants