feat(sc-13): implement merchant registration with MerchantMetadata struct

[diegoucampos-tech]

Description

Implements SC-13: Merchant Registration for the merchant-registry-contract, aligning the contract with the issue specification.

The key change is updating register_merchant to accept a MerchantMetadata struct instead of a bare name: String, enabling richer merchant data to be stored and emitted on-chain.

Type of Change

• New feature

Changes Made

• types.rs — Added MerchantMetadata { name, business_type, contact_info } struct; MerchantInfo now embeds metadata: MerchantMetadata instead of a bare name field
• lib.rs — Updated register_merchant(admin, merchant, metadata) signature per issue spec; added validation for all metadata fields; documented error variants
• errors.rs — Added InvalidMetadata = 7 error variant for business_type/contact_info length validation
• events.rs — MerchantRegistered event now emits full MerchantMetadata payload instead of just a name string

Testing

• Tests pass locally
• New tests added (if applicable)

New tests added (12 total):

• test_registration_flow — full happy path with metadata fields
• test_metadata_stored_correctly — verifies all 3 metadata fields persist correctly
• test_invalid_name_empty — rejects empty name
• test_invalid_name_too_long — rejects name > 64 chars
• test_invalid_business_type_too_long — rejects business_type > 64 chars
• test_invalid_contact_info_too_long — rejects contact_info > 128 chars
• test_merchant_count_increments — verifies count tracking across multiple registrations

Checklist

• Code follows project style guidelines
• Self-review completed
• Comments added for complex code
• Documentation updated
• No new warnings generated

Closes #68

[Josue19-08]

Hey Diego, do you need fix the CIs before to merge

[Josue19-08NC]

Good work overall — the PR description is clear and well-organized, the struct decomposition makes sense, and the test coverage is solid. However there are issues that need to be addressed before this can merge.

CI: Test Contracts is failing

The 'Test Contracts' CI check failed (Build Contracts passed). The PR cannot be merged while tests are failing in CI. Please investigate and fix the failing tests — the locally-passing claim in the description contradicts the CI result.

Missing lower-bound validation on business_type and contact_info

In lib.rs, the name field correctly rejects empty strings (len == 0), but business_type and contact_info only enforce an upper bound:

if metadata.business_type.len() > 64 {
    return Err(Error::InvalidMetadata);
}
if metadata.contact_info.len() > 128 {
    return Err(Error::InvalidMetadata);
}

If both fields are intentionally optional (empty string allowed), that must be documented explicitly in the doc comment and in the PR description. If they are required fields, add a lower-bound check (len == 0) for both, just like name. Right now the intent is ambiguous and the behavior is inconsistent.

No test for empty business_type or contact_info

There is no test that explicitly covers the empty-string case for business_type or contact_info. Whether these should be rejected or accepted, add a test that pins the intended behavior.

Error naming inconsistency

InvalidName (code 5) handles name validation, while InvalidMetadata (code 7) handles business_type and contact_info. Since business_type and contact_info are part of MerchantMetadata and name is also a field of MerchantMetadata, this split is confusing. Consider either renaming InvalidName to something that makes the distinction clearer, or consolidating both under a single error variant with a message. At minimum, the doc comment on the error variants should explain which fields each one covers.

Minor: internal alias is unnecessary

In lib.rs:

use types::{MerchantInfo, MerchantMetadata as MerchantMeta};

MerchantMeta is used only in the register_merchant signature. Since MerchantMetadata is already publicly re-exported, just import it directly without the alias to avoid confusion for readers.

Summary of required changes

1. Fix the failing CI test suite.
2. Clarify and enforce (or document) the lower-bound behavior for business_type and contact_info.
3. Add tests that cover the empty-string case for those two fields.
4. Clarify the error variant naming or add doc comments explaining which fields map to which error.

[Josue19-08]

Good work overall — the PR description is clear, well-organized, and covers the changes thoroughly. The struct decomposition for MerchantMetadata is a clean improvement over the bare name field, and the test suite is comprehensive. However, there are issues that must be resolved before this can merge.

Blocking: CI test suite is failing

The 'Test Contracts' check failed (Build Contracts passed). The PR cannot merge while CI tests are red. The description claims tests pass locally — please investigate the discrepancy and push a fix.

Missing lower-bound validation on business_type and contact_info

In lib.rs, name correctly rejects empty strings, but business_type and contact_info only enforce an upper bound:

if metadata.business_type.len() > 64 { return Err(Error::InvalidMetadata); }
if metadata.contact_info.len() > 128 { return Err(Error::InvalidMetadata); }

This means empty strings are silently accepted for both fields. If they are optional, document that explicitly in the doc comment on the struct fields and in the PR description. If they are required, add a len() == 0 check for each, consistent with how name is handled.

No test pinning empty-string behavior for business_type / contact_info

Whether empty is accepted or rejected, add a test that explicitly covers the empty-string case for both fields. Right now the intended behavior is undocumented and untested.

Unnecessary internal type alias

In lib.rs:

use types::{MerchantInfo, MerchantMetadata as MerchantMeta};

MerchantMeta is only used in the register_merchant signature. MerchantMetadata is already the public name and is re-exported directly below. Remove the alias and import MerchantMetadata directly to avoid confusion for readers.

Error variant naming inconsistency

InvalidName (code 5) covers the name field inside MerchantMetadata, while InvalidMetadata (code 7) covers the other two fields of the same struct. The split is confusing since all three are metadata fields. Consider either renaming InvalidName to make the distinction clearer, or adding doc comments to each error variant explicitly listing which fields each one covers.

Required changes before merge

1. Fix the failing CI test suite.
2. Clarify and enforce (or document) lower-bound behavior for business_type and contact_info.
3. Add tests covering the empty-string case for those two fields.
4. Remove the MerchantMeta alias; import MerchantMetadata directly.
5. Add doc comments to InvalidName and InvalidMetadata clarifying which fields each variant covers.

[Josue19-08]

This PR still has the same issues flagged in the previous two reviews (June 25). No new commits have been pushed since June 19, and CI 'Test Contracts' remains failing. The PR cannot merge in this state.

Blocking: CI test suite is still failing

The 'Test Contracts' check has been failing since the first CI run. No fix has been pushed. The PR must have a green CI run before it can be merged.

Still unresolved from previous reviews

1. Missing lower-bound validation for business_type and contact_info — empty strings are silently accepted. Either reject them (add len() == 0 check consistent with how name is validated) or explicitly document and test that they are optional.
2. No tests covering the empty-string case for business_type or contact_info. Whatever behavior is intended must be pinned by a test.
3. Unnecessary type alias in lib.rs: use types::{MerchantInfo, MerchantMetadata as MerchantMeta}. MerchantMeta is only used in the register_merchant signature. Import MerchantMetadata directly.
4. Error variant naming: InvalidName (code 5) and InvalidMetadata (code 7) both validate fields inside MerchantMetadata. Add doc comments to each variant listing which fields it covers, or consolidate.

Please fix the CI failure first, then address the remaining items, and re-request review.