[Feature] Correct protobuf contract types in Actuator getOwnerAddress()

[halibobo1205]

Problem Statement

The getOwnerAddress() method in the following three Actuators uses incorrect Protobuf types for any.unpack():

Actuator	Location	Current (Wrong) Type	Correct Type
UpdateAssetActuator	L174	AccountUpdateContract	UpdateAssetContract
MarketCancelOrderActuator	L224	AssetIssueContract	MarketCancelOrderContract
MarketSellAssetActuator	L286	AssetIssueContract	MarketSellAssetContract

Impact Analysis

1. Currently not triggered
   The getOwnerAddress() override method on AbstractActuator is not called in production code.

2. Failure mode analysis:
   All three incorrect any.unpack(WrongType.class) calls would throw InvalidProtocolBufferException at runtime. Protobuf's Any.unpack() validates the embedded type_url against the requested class before parsing; a mismatch is rejected immediately. Field-number-based misdecoding (e.g., AccountUpdateContract.account_name at field 1 being read as owner_address) would only apply if raw bytes were parsed directly via WrongType.parseFrom(bytes), which is not the code path here.

3. Code quality: Incorrect type parameters pose a maintenance risk for future development.

This issue was identified during a security audit.

Proposed Solution

Replace the unpack() calls in getOwnerAddress() with the correct Contract type corresponding to each Actuator.

Specification

API Changes: None

Configuration Changes: None

Protocol Changes: None

Scope of Impact

• Core protocol (Actuator layer)

Breaking Changes: None. Only corrects type parameters.

Backward Compatibility: Fully compatible. This method is not called in production code, so the fix does not affect existing behavior.

Implementation

Approach:
Modify the getOwnerAddress() method in each of the three files to use the correct Contract type for unpack(). Minimal change — one line per file.

Willing to implement: Yes

Estimated Complexity: Low (minor changes)

Testing Strategy

Test Scenarios:

• Verify that getOwnerAddress() returns the correct owner address after the fix for all three Actuators
• Run existing unit tests to ensure no regressions

Performance Considerations: None

Additional Context (Optional)

Related Issues/PRs: None

References: Claude AI Scan

[halibobo1205]

This is a minor fix. PR will be submitted shortly.

[lvs0075]

The issue correctly notes that getOwnerAddress() is not called in production code today, which is why this hasn't surfaced as a runtime bug. Before the fix lands, it's worth a quick audit to confirm this assumption still holds:

• Is AbstractActuator.getOwnerAddress() ever invoked reflectively, through a generic actuator dispatch loop, or in any test harness?
• Are there any upcoming PRs or in-flight branches that add a call to this method?

If the method is truly dead code in all paths, it might also be worth adding a unit test that directly invokes getOwnerAddress() on each actuator — not to change behavior, but to act as a regression guard so the next person who does wire this up doesn't reintroduce the wrong type.

Either way, the type correction itself is clearly the right change regardless of reachability.

[halibobo1205]

@lvs0075 Agreed, I will double-check this.

[halibobo1205]

Is it truly dead code?
Yes, in all current paths. The Actuator.getOwnerAddress() interface method is implemented by all 37 concrete actuators but:

• Never called by ActuatorFactory, ActuatorCreator, or TransactionFactory

• Never called in any transaction processing pipeline

• Never invoked via reflection

[waynercheung]

@halibobo1205 @lvs0075
I did a quick audit against the current public tree and want to correct one part of the impact analysis.

The three problematic implementations currently do:

any.unpack(WrongType.class).getOwnerAddress()

Under the current code path, the contract parameter is created via Any.pack(message), so Any.unpack(...) checks the embedded type_url before parsing. If the requested class does not match the packed message type, it throws InvalidProtocolBufferException immediately.

So for these three actuator overrides, if getOwnerAddress() were invoked today, the failure mode would be a type-mismatch exception, not a field-number-based misdecode.

I verified this locally with a focused test:

• Any.pack(UpdateAssetContract) rejects unpack(AccountUpdateContract.class)
• Any.pack(MarketCancelOrderContract) rejects unpack(AssetIssueContract.class)
• Any.pack(MarketSellAssetContract) rejects unpack(AssetIssueContract.class)

A related nuance: field-number analysis is still valid for a different code path, namely directly parsing raw protobuf bytes as the wrong message type. For example, parsing UpdateAssetContract.toByteArray() as AccountUpdateContract would bind field 2 to owner_address, so it would read description, not account_name.
But that is not what Any.unpack(...) does here.

On reachability, in the current public tree I did not find:

• a generic actuator dispatch path calling Actuator#getOwnerAddress()
• a reflective invocation of this method in the repo

The main actuator execution paths call validate() / execute(), and transaction-level owner extraction goes through TransactionCapsule.getOwner() plus TransactionFactory.getContract(...), which is independent of these actuator overrides.

So I still agree the type correction itself is the right fix, but I would update the rationale slightly:

1. In the current public tree, these three getOwnerAddress() overrides do not appear to be used in the main execution path.
2. If they were called today, the wrong Any.unpack(...) type would raise InvalidProtocolBufferException.
3. Adding direct unit tests for these three overrides is still worthwhile as a regression guard once the correct contract types are in place.

[lxcmyf]

I agree this should be fixed, but the key rationale is the Any.unpack(...) type mismatch itself.

In the current code, these three overrides are using the wrong contract class, so if getOwnerAddress() is ever called, the likely failure mode is InvalidProtocolBufferException, not a silent field-number-based misdecode.

So to me this is a low-risk correctness cleanup:

• fix the three unpack() types
• add direct unit tests for these overrides as regression guards

Even if the method is not on the current main execution path, the type correction is still the right change.