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

Various refactors and fixes #381

Merged
merged 31 commits into from
Jun 18, 2024
Merged

Various refactors and fixes #381

merged 31 commits into from
Jun 18, 2024

Conversation

igamigo
Copy link
Collaborator

@igamigo igamigo commented Jun 10, 2024

Most of these were done as part of #361 which was closed. I think we still want to keep them.

mFragaBA and others added 14 commits June 7, 2024 11:25
The fixes can be separated into 2:

- compilation errors (most of them were because the old `Account::new` became `Account::from_parts` and the addition of `OutputNote::Partial` which needed to be handled in some pattern matchings.
- start storing the partial output notes (currently being discarded), this also came with small refactors.
test will fail on retry anyways since the genesis account stays is the same every time.
@mFragaBA mFragaBA force-pushed the mFragaBA-next-0.4 branch 2 times, most recently from 41aa14d to 3680cef Compare June 11, 2024 02:00
@mFragaBA mFragaBA force-pushed the mFragaBA-next-0.4 branch from 3680cef to cd11167 Compare June 11, 2024 02:04
@igamigo igamigo force-pushed the igamigo-refactors branch from d0122c6 to 7929130 Compare June 13, 2024 17:17
@igamigo igamigo marked this pull request as ready for review June 13, 2024 18:48
Copy link
Contributor

@mFragaBA mFragaBA left a comment

Choose a reason for hiding this comment

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

Left some minor questions, but overall looks good!

src/client/sync.rs Show resolved Hide resolved
src/mock.rs Outdated Show resolved Hide resolved
src/tests.rs Show resolved Hide resolved
@mFragaBA mFragaBA force-pushed the mFragaBA-next-0.4 branch 2 times, most recently from 215f26a to e4a2565 Compare June 14, 2024 20:27
Base automatically changed from mFragaBA-next-0.4 to next June 14, 2024 21:37
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was deleted in a previous commit. Do we need it again?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah yes, I had removed it in the other PR and accidentally re-added it here on the merge. Removed again!

Copy link
Contributor

@mFragaBA mFragaBA left a comment

Choose a reason for hiding this comment

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

LGTM! Tested locally and was able to install the cli and run txs as usual.

@igamigo igamigo merged commit 36bd0db into next Jun 18, 2024
7 checks passed
@igamigo igamigo deleted the igamigo-refactors branch June 18, 2024 21:55
bobbinth pushed a commit that referenced this pull request Jul 5, 2024
* feat: point to miden base's next and fix errors

The fixes can be separated into 2:

- compilation errors (most of them were because the old `Account::new` became `Account::from_parts` and the addition of `OutputNote::Partial` which needed to be handled in some pattern matchings.
- start storing the partial output notes (currently being discarded), this also came with small refactors.

* fix: filter out partial notes

* fix: only store in scripts table if output note record has a script

* fix: add new storage type to node config file from #365

* Update the `TransactionAuthenticator` imports

* Remove duplicate `get_falcon_signature`

* fix: update note script root hashes

* Use `&mut rng` for note creation in transactions

* Fix usage of new `InputNote`

* fix: fix after rebase

* cargo: point to node's next branch

* Change node ref

* test: avoid reruns on genesis cli tests

test will fail on retry anyways since the genesis account stays is the same every time.

* add looser sleep times to ensure blocks are included

* test: add helper to wait until notes get committed/consumed

* deps: point to miden-node's next branch and fix compilation errors

* Test transaction ordering

* make: remove dependency for node to avoid duplication on CI

* Rollback node branch

* fix: fix breaking change from base

* fix: update swap note script root

* Various refactors and simplifications

* Lints

* Lints

* Re-add parameter

* Reviews

* Fixes

* Test fix

* Remove file again

---------

Co-authored-by: Martin Fraga <[email protected]>
Co-authored-by: tomyrd <[email protected]>
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