stacks-network / stacks-core

The Stacks blockchain implementation
https://docs.stacks.co
GNU General Public License v3.0
3k stars 659 forks source link

[Stacks 2.1] Lack of synthetic event attached to Delegate Stacks STX Bitcoin op #3465

Closed lgalabru closed 1 year ago

lgalabru commented 1 year ago

It looks like the stacks-node is not attaching a synthetic event when crafting synthetic transactions created for representing Bitcoin operations performing a delegate Stacks STX. This synthetic events are essentials for downstream event observers: transactions can't be classified without these, a typical stacks-api node would block its observed stacks-node, resulting in frozen stacks explorers, etc.
cc @zone117x for confirmation.

zone117x commented 1 year ago

This is also related to https://github.com/stacks-network/stacks-blockchain/issues/2591

The event data given for Bitcoin-chain stacking ops has never included enough information to generate an accurate TX for regular Stack-STX, but we've never seen anyone complain (lack of usage perhaps?).

jcnelson commented 1 year ago

This could be added though. What kind of information do you need? We can already obtain and return anything you'd get from a normal contract-call to stack-stx (or delegate-stx or stx-transfer?), since that's how the system processes it.

zone117x commented 1 year ago

We can already obtain and return anything you'd get from a normal contract-call to stack-stx (or delegate-stx or stx-transfer?), since that's how the system processes it

Does the system generate a contract-call transaction object that can be serialized like a regular contract-call? If so, simply serializing it and including it in the event payload should give the API enough (along with the associated pox2 print events that the normal txs have).

For reference, the current payloads for these tx types have this null byte property coreTx.raw_tx === '0x00', which the API currently detects, then uses the events associated with the txid to generate a synthetic tx:

    if (coreTx.raw_tx === '0x00') {
      const event = allEvents.find(event => event.txid === coreTx.txid);
      if (!event) {
        logger.warn(`Could not find event for process BTC tx: ${JSON.stringify(coreTx)}`);
        return null;
      }
      if (event.type === CoreNodeEventType.StxTransferEvent) {
        rawTx = createTransactionFromCoreBtcTxEvent(chainId, event, coreTx.txid);
        txSender = event.stx_transfer_event.sender;
      } else if (event.type === CoreNodeEventType.StxLockEvent) {
        rawTx = createTransactionFromCoreBtcStxLockEvent(
          chainId,
          event,
          blockData.burn_block_height,
          coreTx.raw_result,
          coreTx.txid
        );
        txSender = event.stx_lock_event.locked_address;
      } else {
        logError(
          `BTC transaction found, but no STX transfer event available to recreate transaction. TX: ${JSON.stringify(
            coreTx
          )}`
        );
        throw new Error('Unable to generate transaction from BTC tx');
      }
    }
friedger commented 1 year ago

Only delegate-stx is a Bitcoin op, not delegate stacks stx. Isn't it? Is there a synthetic event for delegating STX?

janniks commented 1 year ago

Where are the available btc ops defined? I can only find this in SIP-007 (i.e. StackStx and TransferStx)

zone117x commented 1 year ago

Only delegate-stx is a Bitcoin op, not delegate stacks stx. Isn't it?

Yep. The problem is that because these Bitcoin ops are sent to the event-observer with raw_tx === '0x00', the only way to figure out what these txs are supposed to be is by inferring from the associated events. Right now, there are no events associated with the DelegateStx Bitcoin op so it's ignored by the API (and thus won't show up in the explorer and wallets).

Where are the available btc ops defined? I can only find this in SIP-007 (i.e. StackStx and TransferStx)

The DelegateStx Bitcoin op was introduced in 2.1, details here: https://github.com/stacksgov/sips/blob/a7f2e58ec90c12ee1296145562eec75029b89c48/sips/sip-015/sip-015-network-upgrade.md#new-burnchain-transaction-delegate-stx

pavitthrap commented 1 year ago

This is also related to #2591

The event data given for Bitcoin-chain stacking ops has never included enough information to generate an accurate TX for regular Stack-STX, but we've never seen anyone complain (lack of usage perhaps?).

Would an adequate solution to this be to add information that allows transactions to be classified more easily? I could add a field to the transaction event payload, burnchain_op. This field would contain info like the inputs and name of the burn chain operation.

I did something like that in this PR: https://github.com/hirosystems/stacks-subnets/pull/168, and could do that here as well.

@lgalabru @zone117x

pavitthrap commented 1 year ago

Providing more context around "synthetic" events in relation to the bitcoin operations.

The events for the existing btc ops are not synthetic per-se. Transfer stx/ stack stx call the same functions that would be called if they were called as a stacks chain tx. These functions register a STXTransferEvent and STXLockEvent respectively.

Pasted Graphic 1

However, there is not a distinct STX event type for delegate stx; it is executed as function call in the pox-2 contract. So for there to be an event for this function, I would have to create a synthetic transaction. I could create a synthetic print event for it. This solution is perhaps less desirable because then either (a) calls to delegate-stx on the stacks chain would not have a corresponding print events, or (b) I could add synthetic events to calls to delegate-stx from the stacks chain, but that would require some special-casing (there’s already examples special casing for other pox contract calls so not unprecedented).

To avoid the need for synthetic transactions, I could instead provide more information about btc operations in the transaction event payload. This implementation is also generally useful and satisfies an existing request from the API team.

lgalabru commented 1 year ago

Personally in favor of adding a new event type (emitted by both native + bitcoin), as I don't see a verbose system being an issue (specially when it comes to STX tokens status) but I'll let @zone117x chime in, as adding new event types to the system could be an issue for the API.

friedger commented 1 year ago

As a pool admin, I very much like the idea of a new event type or print event that works with stx and btc. Does it also include revoke delegation?

zone117x commented 1 year ago

I think it makes sense to just add another synthetic pox-2 contract print event for the delegate-stx operation, which would get created regardless of if the operation was from a Bitcoin chain operation or a regular Stacks chain contract-call. This is similar to how the existing stack-stx pox-2 print statement works.

At this point I think it makes sense to scope this issue to only the delegate-stx operation (as opposed to tackling a possibly larger issue of improving the event payload for Bitcoin ops in general).

pavitthrap commented 1 year ago

Addressed in #3503