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.

Sign up for GitHub

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

Jump to bottom

fix: address serialization in PayForBlobs event (backport #2793) #2794

Merged
merged 6 commits into from
Nov 6, 2023
Merged

fix: address serialization in PayForBlobs event (backport #2793) #2794

merged 6 commits into from
Nov 6, 2023

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Oct 31, 2023

Backports #2793 to the v1.x branch

@rootulp rootulp self-assigned this Oct 31, 2023
@rootulp
Copy link
Collaborator Author

rootulp commented Oct 31, 2023

I'm testing this via a celestia-stack-internal devnet. Since the validator is still running v1.3.0 and not this patched version, it emits the event with the incorrect address serialization:

    - key: signer
      value: '"celestia1vdjkcetnw35kzvthv43kger6wcurqcmhdv6xcatkxde8gm3jdfcr2utn0p6kz6esvyek2etnwfcqwfh609"'
    type: celestia.blob.v1.EventPayForBlobs
Click to see full logs 
consensus-validator-1-0:~$ celestia-appd query tx C3650B43B14A978E96E6DFD9FFD087E8F20DB930FA6B45AF52DF08EFE2FBBC56 --home /home/celestia
code: 0
codespace: ""
data: 122A0A282F63656C65737469612E626C6F622E76312E4D7367506179466F72426C6F6273526573706F6E7365
events:
- attributes:
  - index: true
    key: c3BlbmRlcg==
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnA=
  - index: true
    key: YW1vdW50
    value: MjEwMDB1dGlh
  type: coin_spent
- attributes:
  - index: true
    key: cmVjZWl2ZXI=
    value: Y2VsZXN0aWExN3hwZnZha20yYW1nOTYyeWxzNmY4NHoza2VsbDhjNWxwbmpzM3M=
  - index: true
    key: YW1vdW50
    value: MjEwMDB1dGlh
  type: coin_received
- attributes:
  - index: true
    key: cmVjaXBpZW50
    value: Y2VsZXN0aWExN3hwZnZha20yYW1nOTYyeWxzNmY4NHoza2VsbDhjNWxwbmpzM3M=
  - index: true
    key: c2VuZGVy
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnA=
  - index: true
    key: YW1vdW50
    value: MjEwMDB1dGlh
  type: transfer
- attributes:
  - index: true
    key: c2VuZGVy
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnA=
  type: message
- attributes:
  - index: true
    key: ZmVl
    value: MjEwMDB1dGlh
  - index: true
    key: ZmVlX3BheWVy
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnA=
  type: tx
- attributes:
  - index: true
    key: YWNjX3NlcQ==
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnAvMQ==
  type: tx
- attributes:
  - index: true
    key: c2lnbmF0dXJl
    value: UStvV2JTR3hPUVM2REVQVmNGUVBaRmZ1SFlDQkRPSCtqR0NlNUtrNWNVbEpPTVkwSFRXK1ZNTVRrdzNaTkh6aWdOdkhwYnBkVi9jaW9ldE43TG1pSFE9PQ==
  type: tx
- attributes:
  - index: true
    key: YWN0aW9u
    value: L2NlbGVzdGlhLmJsb2IudjEuTXNnUGF5Rm9yQmxvYnM=
  type: message
- attributes:
  - index: true
    key: YmxvYl9zaXplcw==
    value: WzFd
  - index: true
    key: bmFtZXNwYWNlcw==
    value: WyJBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUVCQVFFQkFRRUJBUUU9Il0=
  - index: true
    key: c2lnbmVy
    value: ImNlbGVzdGlhMXZkamtjZXRudzM1a3p2dGh2NDNrZ2VyNndjdXJxY21oZHY2eGNhdGt4ZGU4Z20zamRmY3IydXRuMHA2a3o2ZXN2eWVrMmV0bndmY3F3Zmg2MDki
  type: celestia.blob.v1.EventPayForBlobs
gas_used: "67606"
gas_wanted: "210000"
height: "1163"
info: ""
logs:
- events:
  - attributes:
    - key: blob_sizes
      value: '[1]'
    - key: namespaces
      value: '["AAAAAAAAAAAAAAAAAAAAAAAAAAEBAQEBAQEBAQE="]'
    - key: signer
      value: '"celestia1vdjkcetnw35kzvthv43kger6wcurqcmhdv6xcatkxde8gm3jdfcr2utn0p6kz6esvyek2etnwfcqwfh609"'
    type: celestia.blob.v1.EventPayForBlobs
  - attributes:
    - key: action
      value: /celestia.blob.v1.MsgPayForBlobs
    type: message
  log: ""
  msg_index: 0
raw_log: '[{"msg_index":0,"events":[{"type":"celestia.blob.v1.EventPayForBlobs","attributes":[{"key":"blob_sizes","value":"[1]"},{"key":"namespaces","value":"[\"AAAAAAAAAAAAAAAAAAAAAAAAAAEBAQEBAQEBAQE=\"]"},{"key":"signer","value":"\"celestia1vdjkcetnw35kzvthv43kger6wcurqcmhdv6xcatkxde8gm3jdfcr2utn0p6kz6esvyek2etnwfcqwfh609\""}]},{"type":"message","attributes":[{"key":"action","value":"/celestia.blob.v1.MsgPayForBlobs"}]}]}]'
timestamp: "2023-10-31T14:47:54Z"
tx:
  '@type': /cosmos.tx.v1beta1.Tx
  auth_info:
    fee:
      amount:
      - amount: "21000"
        denom: utia
      gas_limit: "210000"
      granter: ""
      payer: ""
    signer_infos:
    - mode_info:
        single:
          mode: SIGN_MODE_DIRECT
      public_key:
        '@type': /cosmos.crypto.secp256k1.PubKey
        key: A7VFd0xwYjNaBL0QeN22WhAp3ALqXqEbRVr9SQKoHY4R
      sequence: "1"
    tip: null
  body:
    extension_options: []
    memo: ""
    messages:
    - '@type': /celestia.blob.v1.MsgPayForBlobs
      blob_sizes:
      - 1
      namespaces:
      - AAAAAAAAAAAAAAAAAAAAAAAAAAEBAQEBAQEBAQE=
      share_commitments:
      - YJe/u+9z8S9G43b0b5SXnhPc9JdEyJR7QwGoq0I507c=
      share_versions:
      - 0
      signer: celestia1wecddzv80cwk4luv3rtn2jp5qsxuak0a3eesrp
    non_critical_extension_options: []
    timeout_height: "0"
  signatures:
  - Q+oWbSGxOQS6DEPVcFQPZFfuHYCBDOH+jGCe5Kk5cUlJOMY0HTW+VMMTkw3ZNHzigNvHpbpdV/cioetN7LmiHQ==

@rootulp
Copy link
Collaborator Author

rootulp commented Nov 1, 2023

When the validator runs this patch, the event gets emitted with the correct signer address. Both consensus nodes remain in sync.

    - key: signer
      value: '"celestia1wecddzv80cwk4luv3rtn2jp5qsxuak0a3eesrp"'
    type: celestia.blob.v1.EventPayForBlobs
Click to see full logs 
consensus-validator-1-0:~$ celestia-appd query tx 3126B907A2FA90D551DFDA2587925D14075E3913196A84156A0F2DB2157AFDE2
code: 0
codespace: ""
data: 122A0A282F63656C65737469612E626C6F622E76312E4D7367506179466F72426C6F6273526573706F6E7365
events:
- attributes:
  - index: true
    key: c3BlbmRlcg==
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnA=
  - index: true
    key: YW1vdW50
    value: MjEwMDB1dGlh
  type: coin_spent
- attributes:
  - index: true
    key: cmVjZWl2ZXI=
    value: Y2VsZXN0aWExN3hwZnZha20yYW1nOTYyeWxzNmY4NHoza2VsbDhjNWxwbmpzM3M=
  - index: true
    key: YW1vdW50
    value: MjEwMDB1dGlh
  type: coin_received
- attributes:
  - index: true
    key: cmVjaXBpZW50
    value: Y2VsZXN0aWExN3hwZnZha20yYW1nOTYyeWxzNmY4NHoza2VsbDhjNWxwbmpzM3M=
  - index: true
    key: c2VuZGVy
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnA=
  - index: true
    key: YW1vdW50
    value: MjEwMDB1dGlh
  type: transfer
- attributes:
  - index: true
    key: c2VuZGVy
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnA=
  type: message
- attributes:
  - index: true
    key: ZmVl
    value: MjEwMDB1dGlh
  - index: true
    key: ZmVlX3BheWVy
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnA=
  type: tx
- attributes:
  - index: true
    key: YWNjX3NlcQ==
    value: Y2VsZXN0aWExd2VjZGR6djgwY3drNGx1djNydG4yanA1cXN4dWFrMGEzZWVzcnAvMg==
  type: tx
- attributes:
  - index: true
    key: c2lnbmF0dXJl
    value: OFZEYXdJMkE5Y0lKNWpNaEo4MFBqcTE0U3QxN1VBWUY1MzZpaS9LS0I4YzZKVmRBeklJZTlyR3B4UUJhVHdoejB5M1NuVndFVDhJZk92WjlzZjFxV0E9PQ==
  type: tx
- attributes:
  - index: true
    key: YWN0aW9u
    value: L2NlbGVzdGlhLmJsb2IudjEuTXNnUGF5Rm9yQmxvYnM=
  type: message
- attributes:
  - index: true
    key: YmxvYl9zaXplcw==
    value: WzFd
  - index: true
    key: bmFtZXNwYWNlcw==
    value: WyJBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFCQWdNRUJRWUhDQWs9Il0=
  - index: true
    key: c2lnbmVy
    value: ImNlbGVzdGlhMXdlY2RkenY4MGN3azRsdXYzcnRuMmpwNXFzeHVhazBhM2Vlc3JwIg==
  type: celestia.blob.v1.EventPayForBlobs
gas_used: "67606"
gas_wanted: "210000"
height: "8256"
info: ""
logs:
- events:
  - attributes:
    - key: blob_sizes
      value: '[1]'
    - key: namespaces
      value: '["AAAAAAAAAAAAAAAAAAAAAAAAAAABAgMEBQYHCAk="]'
    - key: signer
      value: '"celestia1wecddzv80cwk4luv3rtn2jp5qsxuak0a3eesrp"'
    type: celestia.blob.v1.EventPayForBlobs
  - attributes:
    - key: action
      value: /celestia.blob.v1.MsgPayForBlobs
    type: message
  log: ""
  msg_index: 0
raw_log: '[{"msg_index":0,"events":[{"type":"celestia.blob.v1.EventPayForBlobs","attributes":[{"key":"blob_sizes","value":"[1]"},{"key":"namespaces","value":"[\"AAAAAAAAAAAAAAAAAAAAAAAAAAABAgMEBQYHCAk=\"]"},{"key":"signer","value":"\"celestia1wecddzv80cwk4luv3rtn2jp5qsxuak0a3eesrp\""}]},{"type":"message","attributes":[{"key":"action","value":"/celestia.blob.v1.MsgPayForBlobs"}]}]}]'
timestamp: "2023-11-01T12:36:06Z"
tx:
  '@type': /cosmos.tx.v1beta1.Tx
  auth_info:
    fee:
      amount:
      - amount: "21000"
        denom: utia
      gas_limit: "210000"
      granter: ""
      payer: ""
    signer_infos:
    - mode_info:
        single:
          mode: SIGN_MODE_DIRECT
      public_key:
        '@type': /cosmos.crypto.secp256k1.PubKey
        key: A7VFd0xwYjNaBL0QeN22WhAp3ALqXqEbRVr9SQKoHY4R
      sequence: "2"
    tip: null
  body:
    extension_options: []
    memo: ""
    messages:
    - '@type': /celestia.blob.v1.MsgPayForBlobs
      blob_sizes:
      - 1
      namespaces:
      - AAAAAAAAAAAAAAAAAAAAAAAAAAABAgMEBQYHCAk=
      share_commitments:
      - 3VsbHX2DqgyNoefOXvmkujdpgnEg/sVZbd4P0HQG5Xs=
      share_versions:
      - 0
      signer: celestia1wecddzv80cwk4luv3rtn2jp5qsxuak0a3eesrp
    non_critical_extension_options: []
    timeout_height: "0"
  signatures:
  - 8VDawI2A9cIJ5jMhJ80Pjq14St17UAYF536ii/KKB8c6JVdAzIIe9rGpxQBaTwhz0y3SnVwET8IfOvZ9sf1qWA==
txhash: 3126B907A2FA90D551DFDA2587925D14075E3913196A84156A0F2DB2157AFDE2

@rootulp rootulp marked this pull request as ready for review November 1, 2023 12:46
@celestia-bot celestia-bot requested a review from a team November 1, 2023 12:47
@rootulp rootulp mentioned this pull request Nov 1, 2023
Merged
evan-forbes
evan-forbes approved these changes Nov 2, 2023
View reviewed changes
Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

rootulp added a commit that referenced this pull request Nov 6, 2023
@rootulp
fix: address serialization in PayForBlobs event (#2793)
f5fd791 
Closes #2781

Note this _shouldn't_ be consensus breaking b/c the ABCI response
results hash shouldn't depend on events.
[`deterministicResponseDeliverTx`](https://github.com/celestiaorg/celestia-core/blob/ffdb652634fb1b6b6426c0f1a2c5adec8af80253/types/results.go#L45-L55)
filters events from the response. Thanks @cmwaters for identifying.

## Testing

I tested on a Robusta devnet named app-v130-node-v0110. See
#2794 (comment)
and
#2794 (comment)
@rootulp rootulp enabled auto-merge (squash) November 6, 2023 13:47
@rootulp rootulp requested review from staheri14 and rach-id November 6, 2023 13:48
cmwaters
cmwaters approved these changes Nov 6, 2023
View reviewed changes
Copy link
Contributor

@cmwaters cmwaters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 thanks for fixing

@rootulp rootulp merged commit 98d0550 into v1.x Nov 6, 2023
24 of 25 checks passed
@rootulp rootulp deleted the rp/fix-event-signer-v1.x branch November 6, 2023 13:49
0xchainlover pushed a commit to celestia-org/celestia-app that referenced this pull request Aug 1, 2024
@rootulp
fix: address serialization in PayForBlobs event (#2793)
81164a2 
Closes celestiaorg/celestia-app#2781

Note this _shouldn't_ be consensus breaking b/c the ABCI response
results hash shouldn't depend on events.
[`deterministicResponseDeliverTx`](https://github.com/celestiaorg/celestia-core/blob/ffdb652634fb1b6b6426c0f1a2c5adec8af80253/types/results.go#L45-L55)
filters events from the response. Thanks @cmwaters for identifying.

## Testing

I tested on a Robusta devnet named app-v130-node-v0110. See
celestiaorg/celestia-app#2794 (comment)
and
celestiaorg/celestia-app#2794 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evan-forbes evan-forbes evan-forbes approved these changes

@cmwaters cmwaters cmwaters approved these changes

@staheri14 staheri14 Awaiting requested review from staheri14

@rach-id rach-id Awaiting requested review from rach-id

Assignees

@rootulp rootulp

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rootulp @cmwaters @evan-forbes