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

[ADP-3487] Real NetworkEnv implementation #4856

Merged
merged 3 commits into from
Nov 28, 2024
Merged

[ADP-3487] Real NetworkEnv implementation #4856

merged 3 commits into from
Nov 28, 2024

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Nov 22, 2024 •
edited
Loading

  • Add Deposit.IO.Network.NodeToClient module with fromNetworkLayer function
  • Make Cardano.Wallet.Application use a real NetworkEnv, translated with fromNetworkLayer

Comments

Issue Number

ADP-3487

@Anviking Anviking self-assigned this Nov 22, 2024
@Anviking Anviking force-pushed the anviking/ADP-3487/NodeToClient branch 3 times, most recently from 6988f8a to b016522 Compare November 25, 2024 11:12
@paolino paolino force-pushed the anviking/ADP-3487/NodeToClient branch from b016522 to baea517 Compare November 28, 2024 10:26
@Anviking Anviking changed the base branch from master to HeinrichApfelmus/ADP-3488/key-tests November 28, 2024 13:26
@Anviking Anviking force-pushed the anviking/ADP-3487/NodeToClient branch from baea517 to f346460 Compare November 28, 2024 13:26
Anviking
Anviking commented Nov 28, 2024
View reviewed changes
lib/customer-deposit-wallet/src/Cardano/Wallet/Deposit/IO/Network/NodeToClient.hs
fromNetworkLayer nl = mapBlock Read.fromConsensusBlock $
NetworkEnv
{ chainSync = \_tr follower -> do
-- TODO: Connect tracer
Copy link
Member Author

Choose a reason for hiding this comment

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

Should fix

Copy link
Member Author

Choose a reason for hiding this comment

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

For later.

@Anviking Anviking force-pushed the anviking/ADP-3487/NodeToClient branch from 76f957e to 3db0842 Compare November 28, 2024 14:28
Base automatically changed from HeinrichApfelmus/ADP-3488/key-tests to master November 28, 2024 14:49
@Anviking Anviking force-pushed the anviking/ADP-3487/NodeToClient branch from 3db0842 to 295f656 Compare November 28, 2024 16:20
@Anviking Anviking marked this pull request as ready for review November 28, 2024 16:28
@Anviking Anviking requested review from paolino and abailly November 28, 2024 16:28
Anviking
Anviking commented Nov 28, 2024
View reviewed changes
lib/customer-deposit-wallet/src/Cardano/Wallet/Deposit/IO/Network/Mock.hs
, slotToUTCTime = pure Time.unsafeUTCTimeOfSlot
, utcTimeToSlot = pure . Just . Time.unsafeSlotOfUTCTime
Copy link
Member Author

Choose a reason for hiding this comment

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

Unused and dropping for now.

@Anviking Anviking mentioned this pull request Nov 28, 2024
Merged
paolino
paolino approved these changes Nov 28, 2024
View reviewed changes
Copy link
Collaborator

@paolino paolino left a comment

Choose a reason for hiding this comment

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

👍

@Anviking Anviking added this pull request to the merge queue Nov 28, 2024
Merged via the queue into master with commit af8c9d7 Nov 28, 2024
24 checks passed
@Anviking Anviking deleted the anviking/ADP-3487/NodeToClient branch November 28, 2024 18:21
github-merge-queue bot pushed a commit that referenced this pull request Nov 28, 2024
@Anviking
[ADP-3487] Make deposit WalletState more strict (#4862)
2e358ad 
- Make deposit wallet state more strict with StrictData
- Bump cardano-wallet-agda for new increased strictness from
    - cardano-foundation/cardano-wallet-agda#116
    - cardano-foundation/cardano-wallet-agda#117

Depends on #4856 (or tested with it at least, but probably mergeable
without it as well)

## Results

### This PR

~600-800 MB memory usage when syncing with preprod

The heap profile suggests we still have a memory leak, but this is not
immediately observable in the os's activity monitor, so this is a lot
better than before.

![cardano-wallet-strictness-pr](https://github.com/user-attachments/assets/df9ef5c6-c5f5-4d21-963a-9c97dbf8fc82)

### Previously
([anviking/ADP-3487/NodeToClient](https://github.com/cardano-foundation/cardano-wallet/tree/anviking/ADP-3487/NodeToClient)
)

9 GB and growing acording to os's activity monitor:

![cardano-wallet](https://github.com/user-attachments/assets/0e77e794-a439-425f-a686-f77f83e0a741)

### Issue Number

ADP-3494
github-merge-queue bot pushed a commit that referenced this pull request Nov 29, 2024
@Anviking
[ADP-3487] Make deposit WalletState more strict (#4862)
e7547fc 
- Make deposit wallet state more strict with StrictData
- Bump cardano-wallet-agda for new increased strictness from
    - cardano-foundation/cardano-wallet-agda#116
    - cardano-foundation/cardano-wallet-agda#117

Depends on #4856 (or tested with it at least, but probably mergeable
without it as well)

## Results

### This PR

~600-800 MB memory usage when syncing with preprod

The heap profile suggests we still have a memory leak, but this is not
immediately observable in the os's activity monitor, so this is a lot
better than before.

![cardano-wallet-strictness-pr](https://github.com/user-attachments/assets/df9ef5c6-c5f5-4d21-963a-9c97dbf8fc82)

### Previously
([anviking/ADP-3487/NodeToClient](https://github.com/cardano-foundation/cardano-wallet/tree/anviking/ADP-3487/NodeToClient)
)

9 GB and growing acording to os's activity monitor:

![cardano-wallet](https://github.com/user-attachments/assets/0e77e794-a439-425f-a686-f77f83e0a741)

### Issue Number

ADP-3494
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paolino paolino paolino approved these changes

@abailly abailly Awaiting requested review from abailly

Assignees

@Anviking Anviking

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Anviking @paolino