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

Add zkevm to safe models #5758

Merged
merged 57 commits into from
Apr 24, 2024
Merged

Add zkevm to safe models #5758

merged 57 commits into from
Apr 24, 2024

Conversation

danielpartida
Copy link
Contributor

Thank you for contributing to Spellbook!

Thank you for taking the time to submit code in Spellbook. A few things to consider:

  • If you are a first-time contributor, please sign the CLA by copy & pasting exactly what the bot mentions in PR comment
  • Refer to docs section below to answer questions
  • Dune team will review submitted PRs as soon as possible

Best practices

To speed up your development process in PRs, keep these tips in mind:

  • Each commit to your feature branch will rerun CI tests (see example)
    • This includes all modified models on your branch
    • This includes all history of the data
  • Two tips for faster development iteration:
    • Ensure dbt is installed locally (refer to main readme) and run dbt compile
      • This will output raw SQL in target/ directory to copy/paste and run on Dune directly for initial query testing
    • Hardcode a WHERE filter for only ~7 days of history on large source tables, i.e. ethereum.transactions
      • This will speed up the CI tests and output results quicker -- whether that's an error or fully successful run
      • Once comfortable with small timeframe, remove filter and let full history run

Incremental model setup

  • Make sure your unique key columns are exactly the same in the model config block, schema yml file, and seed match columns (where applicable)
  • There cannot be nulls in the unique key columns
    • Be sure to double check key columns are correct or COALESCE() as needed on key column(s), otherwise the tests may fail on duplicates

🪄 Use the built CI tables for testing 🪄

Once CI completes, you can query the CI tables and errors in dune when it finishes running.

  • For example:
    • In the run initial models and test initial models, there will be a schema that looks like this: test_schema.git_dunesql_4da8bae_sudoswap_v2_base_pool_creations
    • This can be temporarily queried in Dune for ~24 hours

Leverage these tables to perform QA testing on Dune query editor -- or even full test dashboards!

Spellbook contribution docs

The docs directory has been implemented to answer as many questions as possible. Please take the time to reference each .md file within this directory to understand how to efficiently contribute & why the repo is designed as it is 🪄

Example questions to be answered:

Please navigate through the docs directory to find as much info as you can.

Note: happy to take PRs to improve the docs, let us know 🤝

@danielpartida
Add v1.4.1 singletons
5444ebf 
L1 singleton 0x41675C099F32341bf84BFc5382aF534df5C7461a
L2 singleton 0x29fcB43b46531BcA003ddC8FCB67FFE91900C76
@danielpartida
Change goerli test
5862466
@danielpartida
Merge branch 'main' of https://github.com/5afe/spellbook
761f15a
@danielpartida
Declare new Safe models for zkevm
8bf1492
@danielpartida
Implement Safe models for zkevm
39d3840
@danielpartida
Update crosschain models with zkevm data
8500052 
Models updated are:
- safe_native_transfers.all.sql -> Added safe_zkevm_matic_transfers
- safe_safes_all.sql -> Added safe_zkevm_safes
- safe_transactions_all.sql -> safe_zkevm_transactions
@dune-eng
Copy link

dune-eng commented Apr 9, 2024

Workflow run id 8616763482 approved.

@dune-eng
Copy link

dune-eng commented Apr 9, 2024

Workflow run id 8616849715 approved.

@dune-eng
Copy link

dune-eng commented Apr 9, 2024

Workflow run id 8616849913 approved.

@dune-eng
Copy link

dune-eng commented Apr 9, 2024

Workflow run id 8616976835 approved.

@dune-eng
Copy link

dune-eng commented Apr 9, 2024

Workflow run id 8616977066 approved.

@dune-eng
Copy link

dune-eng commented Apr 9, 2024

Workflow run id 8617132586 approved.

@dune-eng
Copy link

dune-eng commented Apr 9, 2024

Workflow run id 8617132912 approved.

@danielpartida
Copy link
Contributor Author

gm @jeff-dude,

We are implementing the spells for tracking Safe data on zkevm. Currently, we have an error showing that one of our spells depends on the model zkevm.traces (screenshot attached). However, I think that this model already exists (other screenshot attached). Do you know why we are having this error?

Screenshot 2024-04-09 at 16 15 01

I
Screenshot 2024-04-09 at 16 15 13

@jeff-dude
Copy link
Member

Do you know why we are having this error?

gm daniel. dbt requires you to establish the sources in yml files. please look to add new base sources here:
https://github.com/duneanalytics/spellbook/tree/main/sources/_base_sources

@jeff-dude jeff-dude added the WIP work in progress label Apr 10, 2024
@dune-eng
Copy link

Workflow run id 8627008775 approved.

@dune-eng
Copy link

Workflow run id 8627008824 approved.

@danielpartida
Implement correctly base source
c80852c 
# Conflicts:
#	sources/_base_sources/zkevm_base_sources.yml
@danielpartida
Fix zkevm schema
6e7558f
@dune-eng
Copy link

Workflow run id 8627075190 approved.

@dune-eng
Copy link

Workflow run id 8627075278 approved.

@dune-eng
Copy link

Workflow run id 8633229278 approved.

@dune-eng
Copy link

Workflow run id 8633229382 approved.

@danielpartida
Copy link
Contributor Author

Base model is implemented @jeff-dude

Now facing an error that a decoded smart contract is not found (1st screenshot). But I see in the decoded contracts that the smart contract exists
Screenshot 2024-04-10 at 16 53 23

Screenshot 2024-04-10 at 16 52 40

@jeff-dude jeff-dude added the WIP work in progress label Apr 22, 2024
danielpartida and others added 2 commits April 23, 2024 14:46
@danielpartida @jeff-dude
Update models/safe/zkevm/safe_zkevm_transactions.sql
04f7b8c 
Co-authored-by: jeff-dude <102681548+jeff-dude@users.noreply.github.com>
@danielpartida @jeff-dude
Update models/safe/zkevm/safe_zkevm_transactions.sql
12fbbb7 
Co-authored-by: jeff-dude <102681548+jeff-dude@users.noreply.github.com>
@dune-eng
Copy link

Workflow run id 8800647074 approved.

@dune-eng
Copy link

Workflow run id 8800647247 approved.

@dune-eng
Copy link

Workflow run id 8800649180 approved.

@dune-eng
Copy link

Workflow run id 8800649527 approved.

@dune-eng
Copy link

Workflow run id 8800721358 approved.

@dune-eng
Copy link

Workflow run id 8800721486 approved.

@dune-eng
Copy link

Workflow run id 8800726275 approved.

@dune-eng
Copy link

Workflow run id 8800726757 approved.

@dune-eng
Copy link

Workflow run id 8801274135 approved.

@dune-eng
Copy link

Workflow run id 8801274446 approved.

@danielpartida @jeff-dude
Update models/safe/zkevm/safe_zkevm_schema.yml
bdafa6c 
Co-authored-by: jeff-dude <102681548+jeff-dude@users.noreply.github.com>
@dune-eng
Copy link

Workflow run id 8812118137 approved.

@dune-eng
Copy link

Workflow run id 8812118513 approved.

@dune-eng
Copy link

Workflow run id 8812122551 approved.

@dune-eng
Copy link

Workflow run id 8812123092 approved.

@danielpartida
Copy link
Contributor Author

Thank you for the latest feedback @jeff-dude @Hosuke. Once again, all tests passed. Hopefully we can merge this PR now

jeff-dude
jeff-dude approved these changes Apr 24, 2024
View reviewed changes
Copy link
Member

@jeff-dude jeff-dude 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 the patience, let's get it merged now

@jeff-dude jeff-dude added ready-for-merging and removed WIP work in progress labels Apr 24, 2024
@jeff-dude jeff-dude merged commit 64b3cbc into duneanalytics:main Apr 24, 2024
3 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 24, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Hosuke Hosuke Hosuke approved these changes

@jeff-dude jeff-dude jeff-dude approved these changes

Assignees

@Hosuke Hosuke

@jeff-dude jeff-dude

Labels
ready-for-merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@danielpartida @dune-eng @jeff-dude @Hosuke