Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rewarded_addresses event payload serialization #4492

Closed
zone117x opened this issue Mar 6, 2024 · 7 comments · Fixed by #4545
Closed

Fix rewarded_addresses event payload serialization #4492

zone117x opened this issue Mar 6, 2024 · 7 comments · Fixed by #4545
Assignees
Labels

Comments

@zone117x
Copy link
Member

zone117x commented Mar 6, 2024

Epoch3 introduces the reward_set payload for event observers. It looks like there is some default rust->json serialization going on, which creates a very convoluted JSON payload.

Here's the typescript schema/definition for what we're receiving:

interface PoxRewardSetPayload {
  reward_set: {
    rewarded_addresses: (
      | {
          Standard: [
            {
              bytes: string; // "hex string (no 0x prefix)"
              version: number; // 26, 21
            },
            type: 'SerializeP2PKH' | 'SerializeP2SH'
          ];
        }
      | {
          Addr20: [mainnet: boolean, type: 'P2WPKH', hashbytes: number[]];
        }
      | {
          Addr32: [mainnet: boolean, type: 'P2WSH' | 'P2TR', hashbytes: number[]];
        }
    )[];
  };
}

If it isn't obvious, this schema is obtuse. For reference, here's a snippet of code required to deal with that format (convert one of those entries into a bitcoin address string):

/** Parse a pox reward addr into a bitcoin address */
export function parsePoxSetRewardAddress(
  addr: CoreNodeNewPoxSet['stacker_set']['rewarded_addresses'][0]
): string {
  let poxAddrVersion: PoXAddressVersion;
  let network: 'mainnet' | 'testnet';
  let hashBytes: Uint8Array;
  if ('Standard' in addr) {
    if (addr.Standard[1] === 'SerializeP2PKH') {
      poxAddrVersion = PoXAddressVersion.P2PKH;
    } else if (addr.Standard[1] === 'SerializeP2SH') {
      poxAddrVersion = PoXAddressVersion.P2SH;
    } else {
      throw new Error(`Unknown pox address type: ${addr.Standard[1]}`);
    }
    if (
      addr.Standard[0].version === AddressVersion.MainnetMultiSig ||
      addr.Standard[0].version === AddressVersion.MainnetSingleSig
    ) {
      network = 'mainnet';
    } else if (
      addr.Standard[0].version === AddressVersion.TestnetMultiSig ||
      addr.Standard[0].version === AddressVersion.TestnetSingleSig
    ) {
      network = 'testnet';
    } else {
      throw new Error(`Unknown address version: ${addr.Standard[0].version}`);
    }
    hashBytes = Buffer.from(addr.Standard[0].bytes, 'hex');
  } else if ('Addr20' in addr) {
    network = addr.Addr20[0] ? 'mainnet' : 'testnet';
    if (addr.Addr20[1] === 'P2WPKH') {
      poxAddrVersion = PoXAddressVersion.P2WPKH;
    } else {
      throw new Error(`Unknown pox address type: ${addr.Addr20[1]}`);
    }
    hashBytes = Buffer.from(addr.Addr20[2]);
  } else if ('Addr32' in addr) {
    network = addr.Addr32[0] ? 'mainnet' : 'testnet';
    if (addr.Addr32[1] === 'P2WSH') {
      poxAddrVersion = PoXAddressVersion.P2WSH;
    } else if (addr.Addr32[1] === 'P2TR') {
      poxAddrVersion = PoXAddressVersion.P2TR;
    } else {
      throw new Error(`Unknown pox address type: ${addr.Addr32[1]}`);
    }
    hashBytes = Buffer.from(addr.Addr32[2]);
  } else {
    throw new Error(`Unknown pox address type: ${JSON.stringify(addr)}`);
  }
  const btcAddr = poxAddressToBtcAddress(poxAddrVersion, hashBytes, network);
  return btcAddr;
}

IIUC in the pox clarity contract, these rewarded_addresses are represented as a (version: byte, hashbytes: buffer). In JSON, that would simply be:

interface PoxRewardSetPayloadFixed {
  reward_set: {
    rewarded_addresses: {
      version: number; // byte
      hashbytes: string; // hex string
    }[];
  };
}

The encoded bitcoin address strings would also work, e.g.

interface PoxRewardSetPayloadFixed {
 reward_set: {
   rewarded_addresses: string[]; // bitcoin addresses
  }
}
@hstove
Copy link
Contributor

hstove commented Mar 6, 2024

@zone117x your JS code is right but your TS interface says "reward_set" instead of "stacker_set". Just commenting because it took me a second to see if the Rust code I was looking at was right or not

@zone117x
Copy link
Member Author

zone117x commented Mar 6, 2024

@zone117x your JS code is right but your TS interface says "reward_set" instead of "stacker_set". Just commenting because it took me a second to see if the Rust code I was looking at was right or not

Ah thanks, I think the payload is morphing a bit between when this ingestion code was written and current next / draft PRs :)

@zone117x
Copy link
Member Author

zone117x commented Mar 6, 2024

Also note that I'm currently working off of PR #4430 for this event data

@hstove
Copy link
Contributor

hstove commented Mar 6, 2024

I think this works in event_dispatcher.rs inside process_stacker_set:

        // Convert to a list of b58-encoded strings
        let reward_set_json = json!({
            "rewarded_addresses": reward_set
            .rewarded_addresses
            .iter()
            .cloned()
            .map(|a| a.to_b58())
            .collect::<Vec<String>>(),
            "reward_cycle": reward_set.start_cycle_state,
            "signers": reward_set.signers,
        });

        let payload = json!({
            "stacker_set": reward_set_json,
            "block_id": block_id,
            "cycle_number": cycle_number
        });

@zone117x
Copy link
Member Author

zone117x commented Mar 6, 2024

That .to_b58 always throws me off, because the btc addresses can also be the new(ish) bech32 encoding

@hstove
Copy link
Contributor

hstove commented Mar 6, 2024

Agreed. If we were to write an alias and "deprecate" that fn, what would a better name be?

@zone117x
Copy link
Member Author

zone117x commented Mar 6, 2024

.to_bitcoin_address ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants