Conversation
There was a problem hiding this comment.
Pull request overview
Adds first-class ICON chain support to fkms so the service can derive ICON addresses and expose a gRPC endpoint to sign ICON transactions alongside existing EVM/XRPL support.
Changes:
- Introduces ICON chain type across config, signer address derivation, and gRPC/proto definitions.
- Adds ICON codec utilities and a new
sign_iconservice handler. - Standardizes several error/message format strings to Rust’s
{e}style.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/signer/local.rs | Adds ICON address derivation path and ICON signing dispatch; adds ICON address unit test. |
| src/signer/aws.rs | Adds ICON address derivation for AWS-backed signer initialization. |
| src/signer.rs | Adds public_key_to_icon_address helper using SHA3-256. |
| src/server/service.rs | Adds sign_icon gRPC handler and includes ICON in signer address listing. |
| src/server/middleware/auth.rs | Minor error formatting change. |
| src/main.rs | Minor error formatting change. |
| src/config/signer/local.rs | Extends ChainType with Icon. |
| src/commands/utils.rs | Refactors derivation path formatting. |
| src/codec/tss.rs | Improves test assertion messages formatting. |
| src/codec/icon.rs | New ICON transaction codec (payload creation, encode-for-signing, sign). |
| src/codec.rs | Exposes new icon codec module. |
| proto/fkms/v1/signer.proto | Adds SignIcon RPC + request/response messages; extends proto ChainType. |
| README.md | Documents ICON support and SignIcon API. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds ICON chain support to fkms by extending signer/address derivation, introducing ICON transaction payload encoding/signing, and exposing a new gRPC SignIcon endpoint.
Changes:
- Add
ChainType::Iconsupport across local/AWS signer initialization and signer address listing. - Introduce ICON codec module for building signing payloads, encoding signing bytes, and producing signed tx params.
- Extend gRPC API (
signer.proto) and server implementation to handleSignIconrequests.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/signer/local.rs | Adds ICON address derivation + signing path; adds ICON address unit test. |
| src/signer/aws.rs | Adds ICON address derivation during AWS signer construction. |
| src/signer.rs | Adds public_key_to_icon_address + strengthens EVM pubkey prefix validation. |
| src/server/service.rs | Implements sign_icon gRPC handler and includes ICON in signer listing. |
| src/codec/icon.rs | New ICON tx payload + signing encoding + signature insertion helpers (with a basic test). |
| src/codec.rs | Exposes new codec::icon module. |
| src/config/signer/local.rs | Extends config ChainType enum with Icon. |
| proto/fkms/v1/signer.proto | Adds SignIcon RPC, request/response messages, ICON payload, and enum value. |
| README.md | Updates documentation to mention ICON support and the new gRPC method. |
| src/server/middleware/auth.rs | Minor formatting change in error message interpolation. |
| src/main.rs | Minor formatting change in error printing interpolation. |
| src/commands/utils.rs | Minor string formatting update for BIP44 derivation path. |
| src/codec/tss.rs | Minor test assertion string formatting cleanup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds ICON chain support to FKMS by introducing ICON address derivation, ICON transaction payload encoding/signing, and a new gRPC endpoint for ICON signing.
Changes:
- Add
ChainType::Iconacross config, signer implementations, and gRPC enums. - Implement ICON address derivation and ICON tx signing flow (payload creation → encode-for-signing → SHA3-256 digest → signature embedding).
- Expose
SignIcongRPC method and update docs accordingly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/signer/local.rs | Add ICON key/address derivation and route ICON signing via ECDSA recoverable signatures; add ICON address unit test. |
| src/signer/aws.rs | Restrict AWS signer initialization to EVM only (non-EVM now rejected). |
| src/signer.rs | Add public_key_to_icon_address; tighten EVM pubkey validation (length + SEC1 prefix). |
| src/server/service.rs | Add sign_icon endpoint and include ICON in GetSignerAddresses mapping. |
| src/server/middleware/auth.rs | Formatting-only error message interpolation change. |
| src/main.rs | Formatting-only error printing interpolation change. |
| src/config/signer/local.rs | Add Icon variant to ChainType. |
| src/commands/utils.rs | Minor formatting-only change to HD path string interpolation. |
| src/codec/tss.rs | Formatting-only test assertion message change. |
| src/codec/icon.rs | New ICON codec module: tx struct, signing payload creation, encode-for-signing, and signature embedding (+ tests). |
| src/codec.rs | Export new icon codec module. |
| proto/fkms/v1/signer.proto | Add SignIcon RPC + request/response messages + ICON enum value. |
| README.md | Document ICON support and new SignIcon API; adjust formatting in install section. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds ICON chain support to fkms, extending the signer/address derivation utilities, gRPC surface area, and server-side signing flow to support ICON transaction construction and signing alongside existing EVM/XRPL support.
Changes:
- Add ICON chain type across config, signer utilities, and server signer selection.
- Introduce ICON codec for building/serializing/signing ICON transactions and a new
SignIcongRPC endpoint. - Tighten public-key validation for EVM address derivation and update docs/API definitions accordingly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/signer/local.rs | Adds ICON support for local signer initialization, address derivation, and signing; adds ICON address test. |
| src/signer/aws.rs | Adjusts supported chain handling (now rejects non-EVM). |
| src/signer.rs | Adds ICON address derivation; adds SEC1 prefix validation for EVM pubkeys. |
| src/server/service.rs | Adds sign_icon RPC implementation and includes ICON in signer address listing. |
| src/server/middleware/auth.rs | Minor formatting change in unauthenticated error message. |
| src/main.rs | Minor formatting change in CLI error printing. |
| src/config/signer/local.rs | Adds Icon variant to ChainType. |
| src/commands/utils.rs | Refactors HD path string formatting. |
| src/codec/tss.rs | Improves assertion message formatting in tests. |
| src/codec/icon.rs | New ICON transaction codec (payload creation, signing serialization, signature injection) with unit tests. |
| src/codec.rs | Exposes new icon codec module. |
| proto/fkms/v1/signer.proto | Adds SignIcon RPC, request/response messages, payload type, and ICON enum value. |
| README.md | Documents ICON support and adds proto examples; also changes installation formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds ICON chain support to the FKMS signing service, extending signer/address handling and exposing a new gRPC signing endpoint for ICON alongside existing EVM and XRPL functionality.
Changes:
- Add
ChainType::Iconsupport across config, local signer initialization/signing, and address derivation. - Introduce ICON transaction codec + new
SignIcongRPC API and server handler. - Update docs and modernize some formatting/error-string interpolation.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/config/signer/local.rs | Add Icon variant to ChainType. |
| src/signer.rs | Add public_key_to_icon_address; tighten EVM pubkey prefix validation. |
| src/signer/local.rs | Support ICON local signer init/addressing; route ICON signing to ECDSA recoverable signatures; add ICON address test. |
| src/signer/aws.rs | Restrict AWS signer initialization/signing to EVM only; improve unsupported-chain error detail. |
| src/codec/icon.rs | New ICON tx payload construction, signing preimage encoding, and signature embedding helpers + unit tests. |
| src/codec.rs | Export the new icon codec module. |
| src/server/service.rs | Add SignIcon RPC implementation and include ICON in signer address listing. |
| proto/fkms/v1/signer.proto | Add SignIcon RPC, ICON payload/response messages, and ICON enum value. |
| README.md | Document ICON support and new SignIcon API; adjust installation formatting. |
| Cargo.toml | Make base64 a non-optional dependency; update feature wiring accordingly. |
| src/commands/utils.rs | Minor formatting change for HD derivation path string. |
| src/server/middleware/auth.rs | Update formatting in auth failure message. |
| src/main.rs | Update CLI error printing formatting. |
| src/codec/tss.rs | Update test assertion formatting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds ICON chain support to the Rust fkms service by introducing ICON address derivation, transaction payload/signing codec, and a new gRPC signing endpoint, integrating it into the existing signer/server flow.
Changes:
- Add ICON chain type support in local signer (address derivation + signing) and expose
SignIcongRPC endpoint. - Introduce ICON transaction codec for building signing payloads, encoding for signing, and attaching signatures.
- Update proto/README plus minor formatting/error-message interpolation cleanups; restrict AWS signer to EVM-only.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/signer/local.rs |
Adds ChainType::Icon support (address derivation + ECDSA prehash signing) and ICON address test. |
src/signer/aws.rs |
Removes XRPL handling and returns explicit unsupported-chain errors (EVM-only). |
src/signer.rs |
Adds public_key_to_icon_address and strengthens EVM pubkey validation (0x04 prefix). |
src/server/service.rs |
Adds sign_icon gRPC handler using ICON codec and SHA3-256 prehash signing. |
src/server/middleware/auth.rs |
Minor error string formatting change. |
src/main.rs |
Minor error string formatting change. |
src/config/signer/local.rs |
Adds Icon variant to ChainType. |
src/commands/utils.rs |
Minor string formatting change for BIP44 HD path. |
src/codec/tss.rs |
Minor test assertion message formatting cleanup. |
src/codec/icon.rs |
New ICON transaction codec (payload creation, signing serialization, signature attachment) + tests. |
src/codec.rs |
Exposes new icon codec module. |
proto/fkms/v1/signer.proto |
Adds SignIcon RPC, request/response messages, ICON signer payload, and enum value. |
README.md |
Documents ICON support and new gRPC method; clarifies AWS scope. |
Cargo.toml |
Makes base64 non-optional and adjusts feature definitions accordingly. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| let timestamp = std::time::SystemTime::now() | ||
| .duration_since(std::time::UNIX_EPOCH) | ||
| .context("system time is before UNIX_EPOCH when creating ICON signing payload")? | ||
| .as_micros() as i64; |
There was a problem hiding this comment.
can we try to not cast to i64 ?
No description provided.