Skip to content

refactor(sncast) Introduce accounts repository - #4549

Open
integraledelebesgue wants to merge 1 commit into
feat/sncast-accounts-schema-v2from
refactor/sncast-accounts-repository
Open

refactor(sncast) Introduce accounts repository#4549
integraledelebesgue wants to merge 1 commit into
feat/sncast-accounts-schema-v2from
refactor/sncast-accounts-repository

Conversation

@integraledelebesgue

@integraledelebesgue integraledelebesgue commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Introduced changes

This PR introduces the AccountRepository - a layer between the physical storage of accounts and their corresponding schema-agnostic representations which abstracts filesystem operations from code that reads/modifies the accounts.

It is used later in the stack.

Checklist

  • Linked relevant issue
  • Updated relevant documentation
  • Added relevant tests
  • Performed self-review of the code
  • Added changes to CHANGELOG.md

@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch 4 times, most recently from 9aba5fd to a84dbf7 Compare August 20, 2026 15:12
@integraledelebesgue
integraledelebesgue marked this pull request as ready for review August 20, 2026 15:25
@integraledelebesgue
integraledelebesgue requested a review from a team as a code owner August 20, 2026 15:25
@integraledelebesgue
integraledelebesgue requested review from MKowalski8, ddoktorski and franciszekjob and removed request for MKowalski8 August 20, 2026 15:25
@integraledelebesgue integraledelebesgue changed the title refactor/sncast accounts repository refactor(sncast) Introduce accounts repository Aug 20, 2026

@franciszekjob franciszekjob left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One remark about migration: could we model it more generically (for potential future schema migrations) instead of coupling it specifically to v1 -> latest?

Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/accounts/repository.rs
Comment thread crates/sncast/src/accounts/repository.rs
Comment thread crates/sncast/src/accounts/repository.rs
Comment thread crates/sncast/src/accounts/repository.rs Outdated
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from b0089fd to 8899f7d Compare August 24, 2026 10:04
Comment thread crates/sncast/src/accounts/error.rs Outdated
{
"version": 2,
"accounts": {
"alpha-sepolia": {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about replacing network names in V2 with just chain ids instead of strange alpha-sepolia etc.?

Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/starknet_commands/account/migrate.rs Outdated
Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/accounts/repository.rs Outdated
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 8899f7d to 324d245 Compare August 25, 2026 10:15
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch 2 times, most recently from c17b6d5 to 2b7bd6b Compare August 25, 2026 10:55
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 2b7bd6b to 6ced4c4 Compare August 26, 2026 10:50
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 6ced4c4 to 131b528 Compare August 26, 2026 11:23
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 0483727 to 1ebff2f Compare August 27, 2026 10:23
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 1ebff2f to 466c75b Compare August 27, 2026 11:01
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 466c75b to 139cd08 Compare August 27, 2026 17:11
Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/starknet_commands/account/migrate.rs Outdated
Comment thread crates/sncast/src/response/account/migrate.rs Outdated
Comment thread crates/sncast/src/response/account/migrate.rs Outdated
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 139cd08 to f4f8f3c Compare August 28, 2026 15:04
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch 2 times, most recently from 456142b to d6cb107 Compare August 28, 2026 15:16
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch 2 times, most recently from 69a8a83 to 34a036d Compare August 28, 2026 15:49
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch 3 times, most recently from c67843b to d47015e Compare August 28, 2026 17:16
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch 2 times, most recently from d47015e to 66a75c3 Compare August 31, 2026 13:08
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 66a75c3 to 04c818c Compare September 1, 2026 10:54

@ddoktorski ddoktorski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

PR title seems inaccurate, it is not only refactor as it introduces account migrate command

Comment thread crates/sncast/src/accounts/error.rs Outdated
Comment thread crates/sncast/tests/e2e/account/migrate.rs Outdated
AccountsFile,
TemporaryAccountsFile,
Backup,
ParentDirectory,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is it a variant of FileType?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It simplifies the construction of errors related to files. We don't need to repeat a nearly identical message for every type of file that may produce a given kind of error

Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/tests/e2e/account/migrate.rs Outdated
sync_parent(parent, path)
}

fn with_exclusive_lock<T>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we simplify the code so this function only locks and returns File? (can be renamed to lock_exclusive)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the current implementation is optimal. Returning a plain File is not possible because it needs to be unlocked after performing the operation anyway. We could wrap it with a guard with custom Drop, but it will be a more complex solution than this.

Comment thread crates/sncast/src/accounts/schema/mod.rs Outdated
Comment thread crates/sncast/src/accounts/repository.rs Outdated
Comment thread crates/sncast/src/starknet_commands/account/migrate.rs Outdated
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 04c818c to 83b8799 Compare September 2, 2026 10:48
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 83b8799 to 07b7ed9 Compare September 2, 2026 14:02
@integraledelebesgue
integraledelebesgue force-pushed the refactor/sncast-accounts-repository branch from 07b7ed9 to 8dbb13b Compare September 3, 2026 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants