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

feat(proto, core, sequencer)!: add traceability to rollup deposits #1410

Merged
merged 44 commits into from
Sep 11, 2024

Conversation

ethanoroshiba
Copy link
Contributor

@ethanoroshiba ethanoroshiba commented Aug 27, 2024

Summary

Added id_of_source_transaction and position_in_source_transaction to Deposit.

Background

Previous deposit events had no information about the deposit source, so rollups could not trace if their deposits had landed without checking the balance. Additionally, this could result in deposit txs having the same hash in the EVM.

Changes

  • Created primitive proto type TransactionId for identifying source transaction in deposits.
  • Added id_of_source_transaction and position_in_source_transaction to Deposit.
  • Included tx id in the ephemeral state via current_source() so that it can be accessed by actions.
  • Added accessors/mutators for deposit index to StateReadExt and StateWriteExt traits in transaction.
  • Updated bridge_lock and ics20_transfer actions to populate Deposit events with transaction id and index based on the state.

Testing

Added test to ensure position_in_source_transaction is incremented correctly.

Breaking Changelist

  • App hash changed because of increasing block fees due to longer length of Deposit.
  • Changed SequencerBlock API Deposit event, having two new mandatory fields source_transaction_id and source_action_index. source_transaction_id uses a new primitive proto type TransactionId.

Related Issues

with astriaorg/astria-geth#42, closes #1402

@github-actions github-actions bot added proto pertaining to the Astria Protobuf spec sequencer pertaining to the astria-sequencer crate labels Aug 27, 2024
@ethanoroshiba ethanoroshiba added the core pertaining to the astria-core crate label Aug 27, 2024
Copy link
Member

@SuperFluffy SuperFluffy left a comment

Choose a reason for hiding this comment

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

Thank you for all the work and bearing with my comments. This looks great (and I think we have reduced the new lines by 200).

crates/astria-core/src/sequencerblock/v1alpha1/block.rs Outdated Show resolved Hide resolved
crates/astria-core/src/primitive/v1/mod.rs Outdated Show resolved Hide resolved
@SuperFluffy
Copy link
Member

Please expand on the list of breaking changes explaining why this is breaking the app hash.

@ethanoroshiba ethanoroshiba added this pull request to the merge queue Sep 11, 2024
Merged via the queue into main with commit f543222 Sep 11, 2024
54 checks passed
@ethanoroshiba ethanoroshiba deleted the ENG-744/add_deposit_source branch September 11, 2024 15:24
ethanoroshiba added a commit to astriaorg/astria-geth that referenced this pull request Sep 11, 2024
astriaorg/astria#1410 changes the `Deposit`
message in the `sequencer_block` API, necessitating changes to
`DepositTx` in Geth
github-merge-queue bot pushed a commit that referenced this pull request Sep 11, 2024
## Summary
Update geth tag to `latest` for
astriaorg/astria-geth#42

## Background
#1410 required changes to `astria-geth`, which necessitated changing the
geth dev tag to be the specific PR number. Now that the geth PR is
merged, we can bump the tag.

## Changes
- Changed `evm-rollup` geth dev tag to `latest` and bumped chart
versions.
@ethanoroshiba ethanoroshiba linked an issue Sep 16, 2024 that may be closed by this pull request
steezeburger added a commit that referenced this pull request Sep 23, 2024
* main:
  feat(sequencer): make mempool balance aware (#1408)
  chore(sequencer): change test addresses to versions with known private keys (#1487)
  chore(chart): update geth tag (#1485)
  feat(sequencer): report deposit events (#1447)
  feat(proto, core, sequencer)!: add traceability to rollup deposits (#1410)
  fix(bridge-withdrawer, cli, sequencer-client): migrate from `broadcast_tx_commit` to `broadcast_tx_sync` (#1376)
  fix(sequencer): add `end_block` to `app_execute_transaction_with_every_action_snapshot` (#1455)
  release: end of iteration release cuts (#1456)
  chore(charts): rollupName templates (#1458)
  chore(sequencer-relayer): Add instrumentation (#1375)
  feat(proto, core, sequencer)!: permit bech32 compatible addresses (#1425)
  chore: memoize `address_bytes` of verification key (#1444)
  chore(ci): include `ibc-bridge-test` in `docker` CI target (#1438)
  chore(charts): bump celestia versions (#1431)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cd composer pertaining to composer core pertaining to the astria-core crate proto pertaining to the Astria Protobuf spec sequencer pertaining to the astria-sequencer crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add traceability to rollup deposit source Create newtype for transaction hash
6 participants