Skip to content

refactor/fix: contract objects take two separate providers for reads and writes #110

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
Merged
merged 12 commits into from
Apr 21, 2022
Merged

refactor/fix: contract objects take two separate providers for reads and writes #110

merged 12 commits into from
Apr 21, 2022

Conversation

luketchang
Copy link
Collaborator

@luketchang luketchang commented Apr 20, 2022 •
edited
Loading

  • ethers-rs chore: update ethers to latest master and relock #100 bump fixed updater timelag but did broke tx submission due to fetching timelagged nonces
  • PR creates two separate providers for reads and writes for contract objects
  • Also bumps ethers again to re-include the timelag fix which originally caused the timelag x nonce bug
  • Removes timelag usage for replicas, as we only need it for updater safety on home and processor indexing of Dispatch events

@luketchang luketchang self-assigned this Apr 20, 2022
@luketchang luketchang marked this pull request as ready for review April 20, 2022 22:33
@luketchang
Copy link
Collaborator Author

  • Agents were using timelags for replicas which was causing the processor to be extra slow. We only need timelag for updater safety and processor indexing Dispatch events (since they can be reordered), both of which are only the home. Turned off replica timelag to reach parity with current prod.
  • Use of replica timelags was also causing processor to crash loop when calling prove and process separately because if process failed it would call contract for message status again and timelag would say the message was not proven even though the tip of the chain says it was proven. Fixed by removing replica timelag + fixed processor so proveAndProcess is called not prove then process.

luketchang
luketchang commented Apr 21, 2022
View reviewed changes
chains/nomad-ethereum/src/replica.rs
}
}

#[tracing::instrument(err)]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Implementing here will respect specialized impl

prestwich
prestwich requested changes Apr 21, 2022
View reviewed changes
chains/nomad-ethereum/src/home.rs
write_contract: Arc::new(EthereumHomeInternal::new(
address.as_ethereum_address().expect("!eth address"),
provider.clone(),
write_provider,
Copy link
Member

Choose a reason for hiding this comment

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

use contract.connect to clone with a different provider

https://docs.rs/ethers/latest/ethers/contract/struct.Contract.html#method.connect

Copy link
Member

Choose a reason for hiding this comment

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

nvm: gakonst/ethers-rs#1158

chains/nomad-ethereum/src/replica.rs
}

#[tracing::instrument(err)]
async fn prove_and_process(
Copy link
Member

Choose a reason for hiding this comment

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

was this simply moved? if so, why? if not, what changes were made?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moving to struct impl should fix the problem of the default impl being called (prove then process instead of proveAndProcess)

@luketchang luketchang force-pushed the luke/two-providers branch 4 times, most recently from 3ad18ee to db607dd Compare April 21, 2022 16:50
@luketchang luketchang merged commit 9d8fd31 into main Apr 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@prestwich prestwich prestwich approved these changes

Assignees

@luketchang luketchang

Labels

fix refactor

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@luketchang @prestwich