Add reward_override_entity_key to subscriber mapping activity

[kurotych]

• Handle reward_override_entity_key in SubscriberMappingActivityIngestReportV1 report.
• Add reward_override_entity_key field to the subscriber_mapping_activity table
• Add reward_override_entity_key to SubscriberReward proto
• Handle reward_override_entity_key in reward_index (override the key/address in reward_index table)

[michaeldjeffrey]

A little while back I added a trait AsStringKeyedMap in tests/common/mod.rs to help with things like this.

If you add this impl

impl AsStringKeyedMapKey for SubscriberMappingShares {
    fn key(&self) -> String {
        use helium_proto::Message;
        String::decode(self.subscriber_id.as_bytes()).expect("decode subscriber id")
    }
}

Then you can get at each subscriber directly.

    let sub_map = rewardable_mapping_activity.as_keyed_map();
    let sub_1 = sub_map.get(SUBSCRIBER_1).expect("sub 1");
    let sub_3 = sub_map.get(SUBSCRIBER_3).expect("sub 3");

    assert!(sub_1.reward_override_entity_key.is_none());
    assert_eq!(
        sub_3.reward_override_entity_key,
        Some("entity key".to_string())
    );

[kurotych]

Thank you for pointing me to this solution, it looks really good!

[Copilot]

Pull Request Overview

This PR adds support for a new field, reward_override_entity_key, to the subscriber mapping activity report and related database operations. Key changes include:

• Updating the schema and migration to add reward_override_entity_key.
• Adjusting conversion logic and tests to process empty versus non-empty reward_override_entity_key values.
• Modifying database insert and query operations to include the new field.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
reward_index/tests/integrations/mobile.rs	Added reward_override_entity_key in test payloads.
mobile_verifier/tests/integrations/rewarder_mappers.rs	Updated tests to seed and verify reward_override_entity_key values.
mobile_verifier/tests/integrations/common/mod.rs	Extended key mapping to include SubscriberMappingShares.
mobile_verifier/src/subscriber_mapping_activity/db.rs	Updated save and rewardable_mapping_activity functions to incorporate the field.
mobile_verifier/src/subscriber_mapping_activity.rs	Added the reward_override_entity_key field to the struct and its conversion logic.
mobile_verifier/src/reward_shares.rs	Mapped reward_override_entity_key with a default value transformation.
mobile_verifier/migrations/51_add_ent_key_to_subscr_map_act.sql	Added migration for the new column.
Cargo.toml	Updated dependency branch for helium-proto.

Comments suppressed due to low confidence (1)

mobile_verifier/src/reward_shares.rs:233

• Ensure that defaulting a missing reward_override_entity_key to an empty string is the intended behavior across all contexts, as it might mask the absence of a value.

reward_override_entity_key: mas.reward_override_entity_key.unwrap_or("".to_string()),

[Copilot]

[nitpick] Consider using report.reward_override_entity_key.trim().is_empty() to also handle cases where the string may contain only whitespace.

Suggested change

	let reward_override_entity_key = if report.reward_override_entity_key.is_empty() {
	let reward_override_entity_key = if report.reward_override_entity_key.trim().is_empty() {

[jeffgrunewald]

ALTER TABLE IF EXISTS subscriber_mapping_activity ADD COLUMN IF NOT EXISTS reward_override_entity_key TEXT;

i find this defensive style of migration causes less friction when doing local dev and testing on a db i don't necessarily want to have to wipe and reload every time.

[jeffgrunewald]

Suggested change

	.unwrap_or("".to_string()),
	.unwrap_or_default(),

will result in an empty string

[jeffgrunewald]

Suggested change

	let key = if r.reward_override_entity_key.is_empty() {
	let key = if r.reward_override_entity_key.trim().is_empty() {

[michaeldjeffrey]

duplicate checks?

[kurotych]

yeap, good catch

[bbalser]

should we validate that the reward_override_entity_key is a valid on-chain entity if set?

[kurotych]

should we validate that the reward_override_entity_key is a valid on-chain entity if set

I think it could be a good defense against typos or a compromised service.
At which step (service) do you think it would be better to implement it?

[bbalser]

should we validate that the reward_override_entity_key is a valid on-chain entity if set

I think it could be a good defense against typos or a compromised service. At which step (service) do you think it would be better to implement it?

oracles/mobile_verifier/src/subscriber_mapping_activity.rs

Line 180 in 5365699

async fn verify_activity<AV, EV>(