Skip to content

Add signer address for endorsement calls#594

Merged
zupzup merged 1 commit into
masterfrom
581-add-signer-address
Jul 17, 2025
Merged

Add signer address for endorsement calls#594
zupzup merged 1 commit into
masterfrom
581-add-signer-address

Conversation

@zupzup
Copy link
Copy Markdown
Collaborator

@zupzup zupzup commented Jul 16, 2025

User description

📝 Description

  • Adds the address to the signer for endorsements and past endorsees call as requested by FE

Relates to #581


✅ 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 a bill with several endorsements, the signer should have an address

🤝 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?

PR Type

Enhancement


Description

  • Add signer address to endorsement API calls

  • Update data structures to include address information

  • Modify web bindings for frontend compatibility


Changes diagram

flowchart LR
  A["LightBillIdentParticipant"] --> B["LightBillIdentParticipantWithAddress"]
  B --> C["Web Bindings Update"]
  C --> D["Frontend API Enhancement"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
mod.rs
Update participant data structure with address                     

crates/bcr-ebill-core/src/contact/mod.rs

  • Update LightBillParticipant enum to use
    LightBillIdentParticipantWithAddress
  • Replace LightBillIdentParticipant with address-enabled variant
  • +1/-1     
    bill.rs
    Update web bindings for address support                                   

    crates/bcr-ebill-wasm/src/data/bill.rs

  • Update web bindings for LightBillParticipantWeb enum
  • Change Ident variant to use LightBillIdentParticipantWithAddressWeb
  • +1/-1     
    Documentation
    CHANGELOG.md
    Document signer address feature addition                                 

    CHANGELOG.md

  • Add changelog entry for signer address feature
  • Document API enhancement for endorsements and past endorsees
  • +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @qodo-code-review
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ✅

    581 - PR Code Verified

    Compliant requirements:

    • Add address to signer in endorsements response when signer is Ident type
    • Add address to signer in past endorsees response when signer is Ident type

    Requires further human verification:

    • Verify that endorsements API actually returns signer with address
    • Verify that past_endorsees API actually returns signer with address
    • Test with actual bill creation and endorsement flow

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Missing Implementation

    The code changes the enum variant but doesn't show the actual implementation or usage of LightBillIdentParticipantWithAddress. Need to verify this struct exists and has the required address field.

        Ident(LightBillIdentParticipantWithAddress),
    }
    Type Consistency

    The web binding conversion assumes LightBillIdentParticipantWithAddressWeb exists and properly converts the address field. Need to verify the web types are properly defined and the conversion is complete.

        Ident(LightBillIdentParticipantWithAddressWeb),
    }
    
    impl From<LightBillParticipant> for LightBillParticipantWeb {
        fn from(val: LightBillParticipant) -> Self {
            match val {
                LightBillParticipant::Ident(data) => LightBillParticipantWeb::Ident(data.into()),

    Copy link
    Copy Markdown
    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 extends participant types to include signer addresses for endorsement-related API calls.

    • Replaces existing LightBillIdentParticipantWeb with a new LightBillIdentParticipantWithAddressWeb in the WASM data model
    • Updates the core LightBillParticipant::Ident variant to carry address data
    • Adds a CHANGELOG entry for the new signer address field

    Reviewed Changes

    Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

    File Description
    crates/bcr-ebill-wasm/src/data/bill.rs Updated LightBillParticipantWeb enum to use the WithAddress variant
    crates/bcr-ebill-core/src/contact/mod.rs Modified LightBillParticipant enum to include address in Ident
    CHANGELOG.md Documented the addition of the signer address for endorsement calls
    Comments suppressed due to low confidence (3)

    crates/bcr-ebill-core/src/contact/mod.rs:164

    • [nitpick] Consider renaming the enum variant from Ident to IdentWithAddress to more clearly communicate that this variant now includes an address field and improve consistency between the variant name and its payload type.
        Ident(LightBillIdentParticipantWithAddress),
    

    crates/bcr-ebill-wasm/src/data/bill.rs:852

    • Add or update unit tests covering the LightBillIdentParticipantWithAddressWeb variant to verify that the new address field is correctly serialized and deserialized in endorsement-related API calls.
    pub enum LightBillParticipantWeb {
    

    crates/bcr-ebill-wasm/src/data/bill.rs:854

    • Ensure the From<LightBillParticipant> implementation is updated to map the old Ident variant to LightBillIdentParticipantWithAddressWeb, including passing through the address field. Without this change, conversions will either fail to compile or omit the address.
        Ident(LightBillIdentParticipantWithAddressWeb),
    

    @qodo-code-review
    Copy link
    Copy Markdown

    qodo-code-review Bot commented Jul 16, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @codecov
    Copy link
    Copy Markdown

    codecov Bot commented Jul 16, 2025

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    📢 Thoughts on this report? Let us know!

    @zupzup zupzup force-pushed the 581-add-signer-address branch from bd1023a to b7c10e5 Compare July 17, 2025 06:58
    @zupzup zupzup merged commit 25c00bf into master Jul 17, 2025
    6 checks passed
    @zupzup zupzup deleted the 581-add-signer-address branch July 17, 2025 08:04
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    5 participants