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

refactor: create senders based on escrow account version #624

Merged
merged 9 commits into from
Feb 12, 2025
Merged

refactor: create senders based on escrow account version #624

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
75d1b7d
refactor: create senders based on escrow account
gusinacio Feb 11, 2025
033cdaa
refactor: implement get_pending_sender_allocation_id_v2
gusinacio Feb 11, 2025
342fad5
test: update test-log to print debug messages
gusinacio Feb 12, 2025
2ba1960
test: update create_manager() to receive list of initial accounts
gusinacio Feb 12, 2025
4f28961
refactor: pass sender_type to sender_account
gusinacio Feb 12, 2025
4bded36
chore: add just ci cmd
gusinacio Feb 12, 2025
da09924
test: add back where last
gusinacio Feb 12, 2025
d70e7e2
docs: add just ci docs
gusinacio Feb 12, 2025
50b7dc4
perf: update queries to load up v2
gusinacio Feb 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...3dabb56d7b947b023ee3a8b32b24b07bcd18.json → ...cc1fe1e5367bd545ca489d116de0a709a6ee.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions .sqlx/query-7124365390787371768fdb52259f8a9fd7f1a549ddd6410fa4359cacbea2ce6e.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 26 additions & 0 deletions .sqlx/query-7d2c582d3f92ccf179847bd01de43a328bea75b556032bfea57eb61b30e51f95.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 23 additions & 2 deletions crates/monitor/src/escrow_accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,33 @@ pub async fn escrow_accounts(
reject_thawing_signers: bool,
) -> Result<EscrowAccountsWatcher, anyhow::Error> {
indexer_watcher::new_watcher(interval, move || {
get_escrow_accounts(escrow_subgraph, indexer_address, reject_thawing_signers)
get_escrow_accounts_v1(escrow_subgraph, indexer_address, reject_thawing_signers)
})
.await
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this function should be renamed to escrow_accounts_v1, it can make it a bit confusing? since in every other spot you are making the difference in the names between v1 and v2 and this could give the idea is generic for both versions

Copy link
Contributor Author

@gusinacio gusinacio Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

You are absolutely right, I just didn't want to change this because it would have tons of other changes (since it's also used in service).

I could I do this renaming in a follow-up PR, unless you think it's mandatory.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe just create an issue so that this is not forgotten?


async fn get_escrow_accounts(
pub async fn escrow_accounts_v2(
escrow_subgraph: &'static SubgraphClient,
indexer_address: Address,
interval: Duration,
reject_thawing_signers: bool,
) -> Result<EscrowAccountsWatcher, anyhow::Error> {
indexer_watcher::new_watcher(interval, move || {
get_escrow_accounts_v2(escrow_subgraph, indexer_address, reject_thawing_signers)
})
.await
}

// TODO implement escrow accounts v2 query
async fn get_escrow_accounts_v2(
_escrow_subgraph: &'static SubgraphClient,
_indexer_address: Address,
_reject_thawing_signers: bool,
) -> anyhow::Result<EscrowAccounts> {
Ok(EscrowAccounts::new(HashMap::new(), HashMap::new()))
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add TODO: Change query for temporary implementation of subgraph handling both v1 and V2

Copy link
Contributor Author

@gusinacio gusinacio Feb 12, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

There's already a TODO on top of the function. Do you want me to change the message?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I just tried to mean that the current comment kind of only sounds for v2 and we might need to modify also v1 query later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment you change the provided schema old queries will break so you will quickly realize that you need to change the query.

On the other hand, this won't break and that's why there's a TODO

async fn get_escrow_accounts_v1(
escrow_subgraph: &'static SubgraphClient,
indexer_address: Address,
reject_thawing_signers: bool,
Expand Down
3 changes: 2 additions & 1 deletion crates/monitor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub use crate::{
deployment_to_allocation::{deployment_to_allocation, DeploymentToAllocationWatcher},
dispute_manager::{dispute_manager, DisputeManagerWatcher},
escrow_accounts::{
escrow_accounts, EscrowAccounts, EscrowAccountsError, EscrowAccountsWatcher,
escrow_accounts, escrow_accounts_v2, EscrowAccounts, EscrowAccountsError,
EscrowAccountsWatcher,
},
};
2 changes: 1 addition & 1 deletion crates/tap-agent/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,6 @@ tempfile = "3.8.0"
wiremock.workspace = true
wiremock-grpc = "0.0.3-alpha3"
test-assets = { path = "../test-assets" }
test-log = { version = "0.2.12", default-features = false }
test-log = { version = "0.2.12", features = ["trace"] }
bon = "3.3"
rstest = "0.24.0"
18 changes: 15 additions & 3 deletions crates/tap-agent/src/agent.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ use indexer_config::{
Config, EscrowSubgraphConfig, GraphNodeConfig, IndexerConfig, NetworkSubgraphConfig,
SubgraphConfig, SubgraphsConfig, TapConfig,
};
use indexer_monitor::{escrow_accounts, indexer_allocations, DeploymentDetails, SubgraphClient};
use indexer_monitor::{
escrow_accounts, escrow_accounts_v2, indexer_allocations, DeploymentDetails, SubgraphClient,
};
use ractor::{concurrency::JoinHandle, Actor, ActorRef};
use sender_account::SenderAccountConfig;
use sender_accounts_manager::SenderAccountsManager;
Expand Down Expand Up @@ -154,7 +156,16 @@ pub async fn start_agent() -> (ActorRef<SenderAccountsManagerMessage>, JoinHandl
.await,
));

let escrow_accounts = escrow_accounts(
let escrow_accounts_v1 = escrow_accounts(
escrow_subgraph,
*indexer_address,
*escrow_sync_interval,
false,
)
.await
.expect("Error creating escrow_accounts channel");

let escrow_accounts_v2 = escrow_accounts_v2(
escrow_subgraph,
*indexer_address,
*escrow_sync_interval,
Expand All @@ -170,7 +181,8 @@ pub async fn start_agent() -> (ActorRef<SenderAccountsManagerMessage>, JoinHandl
domain_separator: EIP_712_DOMAIN.clone(),
pgpool,
indexer_allocations,
escrow_accounts,
escrow_accounts_v1,
escrow_accounts_v2,
escrow_subgraph,
network_subgraph,
sender_aggregator_endpoints: sender_aggregator_endpoints.clone(),
Expand Down
Loading
Loading