Skip to content

fix(ttx): avoid GetSigner for remote identities via IsMe gate#1564

Open
atharrva01 wants to merge 2 commits into
hyperledger-labs:mainfrom
atharrva01:fix/collect-endorsements-signer-is-me-gate
Open

fix(ttx): avoid GetSigner for remote identities via IsMe gate#1564
atharrva01 wants to merge 2 commits into
hyperledger-labs:mainfrom
atharrva01:fix/collect-endorsements-signer-is-me-gate

Conversation

@atharrva01
Copy link
Copy Markdown
Contributor

Closes #1226

Every token transfer has a list of signer identities -- some local, some remote. Before this fix, requestSignatures() was calling GetSigner() on every identity unconditionally.

For regular X.509 identities that's a cheap failure. For idemix pseudonyms it's not: GetSigner() internally calls DeserializeSigningIdentity(), which generates a real cryptographic signature as a liveness check -- even for remote identities that will just fail anyway. In a multi-party transfer, this adds up.

The fix

I gated GetSigner() behind IsMe():

// Before
if signer, err := sigService.GetSigner(ctx, signerIdentity); err == nil { … }

// After
if sigService.IsMe(ctx, signerIdentity) {
    signer, err := sigService.GetSigner(ctx, signerIdentity)
    …
}

IsMe() uses an in-memory cache + lightweight DB lookup -- no crypto, no key deserialization. Remote identities are filtered out before any crypto is touched. The rest of the flow (owner-wallet check, remote signing) is unchanged.

Testing

Added TestRequestSignatures_RemoteIdentity_SkipsGetSigner -- sets up a remote identity, configures IsMe() to return false, and asserts GetSigner call count stays at zero. All existing token/services/ttx/... tests pass.

@atharrva01
Copy link
Copy Markdown
Contributor Author

hi @adecaro , I gated GetSigner() behind IsMe() to skip unnecessary idemix crypto costs for remote identities.

@adecaro adecaro force-pushed the fix/collect-endorsements-signer-is-me-gate branch from 59e2fcc to f5bf435 Compare April 23, 2026 12:50
@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented Apr 23, 2026

Hi @atharrva01 , thank you for submitting this and sorry for the late reply 🙏

I'll review ASAP. I'm curious to look at your approach more carefully 😄

@adecaro adecaro self-assigned this Apr 23, 2026
@adecaro adecaro added this to the Q2/26 milestone Apr 23, 2026
atharrva01 added 2 commits May 4, 2026 11:46
…edger-labs#1226)

Gate GetSigner() behind a cheap IsMe() check to skip the expensive
idemix sign-and-verify deserialization for remote identities.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>
… testifylint

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>
@adecaro adecaro force-pushed the fix/collect-endorsements-signer-is-me-gate branch from f5bf435 to 6417f23 Compare May 4, 2026 09:46
@adecaro
Copy link
Copy Markdown
Contributor

adecaro commented May 4, 2026

Hi @atharrva01 , apologies for this late reply. I just want to double check the semantic.
I'm not sure if we are solving the problem. That code that uses the signatures might still be needed.

@atharrva01
Copy link
Copy Markdown
Contributor Author

atharrva01 commented May 4, 2026

Hi @adecaro! You're right to flag this.

The issue is that IsMe() relies on GetExistingSignerInfo, which only tracks identities where StoreSignerInfo was previously called, and that only happens inside a successful GetSigner(). RegisterIdentityDescriptor (called at wallet init) writes to a separate table and populates the in-memory cache, but not the signer-info row. So after a restart, IsMe() can return false for a locally-owned identity that hasn't been through GetSigner() in the current process, causing the gate to skip local signing incorrectly.

I fixed it by calling StoreSignerInfo inside RegisterIdentityDescriptor when the descriptor has a signer, so IsMe() stays accurate across restarts.

Also, for idemixnym identities specifically, DeserializeSigner calls GetSignerInfo first and fails fast before any crypto for remote identities, so the expensive sign-and-verify was only being hit for raw idemix, not pseudonyms. The gate still cuts that cost.

All identity and ttx tests pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve CollectEndorsementsView's handling of signers

2 participants