Skip to content

Fix signing address and signatory for anon signers#849

Open
zupzup wants to merge 1 commit intomasterfrom
840-fix-signing-address-anon-signer
Open

Fix signing address and signatory for anon signers#849
zupzup wants to merge 1 commit intomasterfrom
840-fix-signing-address-anon-signer

Conversation

@zupzup
Copy link
Collaborator

@zupzup zupzup commented Mar 6, 2026

📝 Description

  • If the bill holder is anon, the signing_address needs to not be set
  • If the bill holder is anon and a company, the signatory shouldn't have a name set (breaking DB and bill block change)

Relates to #840


✅ Checklist

Please ensure the following tasks are completed before requesting a review:

  • My code adheres to the coding guidelines of this project.
  • I have run cargo fmt.
  • I have run cargo clippy.
  • I have added or updated tests (if applicable).
  • All CI/CD steps were successful.
  • I have updated the documentation (if applicable).
  • I have checked that there are no console errors or warnings.
  • I have verified that the application builds without errors.
  • I've described the changes made to the API. (modification, addition, deletion).

🚀 Changes Made

See above.


💡 How to Test

Please provide clear instructions on how reviewers can test your changes:

  1. cargo test
  2. create bill - endorse to company as bearer - endorse back, signatory name should be None, signing_address should be None

🤝 Related Issues

List any related issues, pull requests, or discussions:


📋 Review Guidelines

Please focus on the following while reviewing:

  • Does the code follow the repository's contribution guidelines?
  • Are there any potential bugs or performance issues?
  • Are there any typos or grammatical errors in the code or comments?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes privacy leakage issues when anonymous (bearer) bill holders perform bill actions. Previously, the signing_address and signatory.name fields were being populated even when the holder was anonymous, which could reveal identity information. The PR introduces a new LightBillSignatory type with an optional name field to replace the previous LightBillIdentParticipant usage for signatories, and adds logic to omit the signing address and signatory name when the holder is anonymous.

Changes:

  • Changed BillSignatoryBlockData.name and BillSignatory.name from Name to Option<Name>, propagating the type change through all layers (core, persistence DB, WASM)
  • Added signatory_for_signer() helper and conditional signing_address logic in block construction to clear identity data for anonymous holders
  • Added a new integration test endorse_bitcredit_bill_anon_company_and_back that verifies both the signatory name and signing address are None for anonymous company signers

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Cargo.toml Version bump from 0.5.3 to 0.5.4
CHANGELOG.md Added changelog entry for 0.5.4
crates/bcr-ebill-core/src/protocol/blockchain/bill/block.rs Changed BillSignatoryBlockData.name to Option<Name> (breaking borsh change); updated all test fixtures
crates/bcr-ebill-core/src/protocol/blockchain/bill/participant.rs Changed BillSignatory.name to Option<Name> with updated doc comment
crates/bcr-ebill-core/src/application/contact/mod.rs Added new LightBillSignatory struct with Option<Name>
crates/bcr-ebill-core/src/application/bill/mod.rs Changed LightSignedBy.signatory to use LightBillSignatory; removed ContactType import
crates/bcr-ebill-core/src/application/identity/mod.rs Removed From<Identity> for BillSignatoryBlockData impl (no longer 1:1 due to Option<Name>)
crates/bcr-ebill-api/src/service/bill_service/blocks.rs Added signatory_for_signer() helper; applied anon checks to signing_address and signatory across all anon-capable bill actions
crates/bcr-ebill-api/src/service/bill_service/data_fetching.rs Replaced removed From impl with explicit BillSignatoryBlockData construction
crates/bcr-ebill-api/src/service/bill_service/tests.rs Added endorse_bitcredit_bill_anon_company_and_back test; updated existing test fixture
crates/bcr-ebill-persistence/src/db/bill.rs Added LightBillSignatoryDb struct; updated LightSignedByDb to use it
crates/bcr-ebill-wasm/src/data/bill.rs Added LightBillSignatoryWeb struct; updated LightSignedByWeb to use it

@codecov
Copy link

codecov bot commented Mar 6, 2026

Codecov Report

❌ Patch coverage is 84.15301% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.61%. Comparing base (c8f433e) to head (7a49e68).

Files with missing lines Patch % Lines
crates/bcr-ebill-persistence/src/db/bill.rs 0.00% 16 Missing ⚠️
crates/bcr-ebill-wasm/src/data/bill.rs 0.00% 7 Missing ⚠️
...s/bcr-ebill-api/src/service/bill_service/blocks.rs 90.76% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #849      +/-   ##
==========================================
+ Coverage   70.46%   70.61%   +0.15%     
==========================================
  Files         132      132              
  Lines       24802    24942     +140     
==========================================
+ Hits        17477    17614     +137     
- Misses       7325     7328       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@zupzup zupzup force-pushed the 840-fix-signing-address-anon-signer branch from 43b2e79 to 7a49e68 Compare March 6, 2026 14:50
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