feat: dedicated payment addresses (PLT-5296) #88
Conversation
WalkthroughThe changes introduce a new module for handling profile wallets, including address assignment and synchronization. The server now includes an address rotation mechanism and the minimum amount for address assignment is configurable. Error handling has been improved in several places and the test suite has been expanded to cover the new functionality. Changes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files ignored due to filter (1)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/example.json
Files selected for processing (23)
- client/Main.hs (3 hunks)
- dapps-certification-persistence/dapps-certification-persistence.cabal (1 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence.hs (2 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/API.hs (5 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/Structure.hs (9 hunks)
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1 hunks)
- nix/materialized/aarch64-darwin/.plan.nix/dapps-certification-persistence.nix (1 hunks)
- nix/materialized/aarch64-darwin/.plan.nix/plutus-certification.nix (3 hunks)
- nix/materialized/x86_64-darwin/.plan.nix/dapps-certification-persistence.nix (1 hunks)
- nix/materialized/x86_64-darwin/.plan.nix/plutus-certification.nix (3 hunks)
- nix/materialized/x86_64-linux/.plan.nix/dapps-certification-persistence.nix (1 hunks)
- nix/materialized/x86_64-linux/.plan.nix/plutus-certification.nix (3 hunks)
- plutus-certification.cabal (2 hunks)
- server/Main.hs (4 hunks)
- src/Plutus/Certification/API/Routes.hs (3 hunks)
- src/Plutus/Certification/API/Swagger.hs (1 hunks)
- src/Plutus/Certification/CertificationBroadcaster.hs (2 hunks)
- src/Plutus/Certification/Internal.hs (3 hunks)
- src/Plutus/Certification/ProfileWallet.hs (1 hunks)
- src/Plutus/Certification/Server/Instance.hs (7 hunks)
- src/Plutus/Certification/Server/Internal.hs (5 hunks)
- src/Plutus/Certification/Synchronizer.hs (10 hunks)
- src/Plutus/Certification/WalletClient.hs (6 hunks)
Files skipped from review due to trivial changes (12)
- dapps-certification-persistence/dapps-certification-persistence.cabal
- dapps-certification-persistence/src/IOHK/Certification/Persistence/Structure.hs
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs
- nix/materialized/aarch64-darwin/.plan.nix/dapps-certification-persistence.nix
- nix/materialized/aarch64-darwin/.plan.nix/plutus-certification.nix
- nix/materialized/x86_64-darwin/.plan.nix/dapps-certification-persistence.nix
- nix/materialized/x86_64-linux/.plan.nix/dapps-certification-persistence.nix
- plutus-certification.cabal
- src/Plutus/Certification/API/Swagger.hs
- src/Plutus/Certification/Internal.hs
- src/Plutus/Certification/Server/Internal.hs
- src/Plutus/Certification/ProfileWallet.hs (diff too large)
Review comments generated (9)
- Actionable : 9
- LGTM (Skipped): 5
- Noise Suppression (Skipped): 31
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
server/Main.hs
Outdated
| @@ -80,6 +85,7 @@ data Args = Args | |||
| , auth :: !AuthMode | |||
| , signatureTimeout :: !Seconds | |||
| , useWhitelist :: !Bool | |||
| , minAmountForAddressAssessment :: !Word64 | |||
There was a problem hiding this comment.
A new field minAmountForAddressAssessment has been added to the Args data type. Make sure this field is properly initialized when creating an instance of Args and used correctly where needed.
server/Main.hs
Outdated
| addressRotation <- liftIO $ newMVar emptyAddressRotation | ||
| _ <- initDb | ||
| _ <- forkIO $ startTransactionsMonitor (narrowEventBackend InjectSynchronizer eb) (args.wallet) 10 | ||
| _ <- forkIO $ startTransactionsMonitor | ||
| (narrowEventBackend InjectSynchronizer eb) | ||
| (args.wallet) 10 (args.minAmountForAddressAssessment) | ||
| -- TODO: this has to be refactored | ||
| runSettings settings . application (narrowEventBackend InjectServeRequest eb) $ | ||
| cors (const $ Just corsPolicy) . | ||
| serveWithContext (Proxy @APIWithSwagger) (genAuthServerContext whitelist args.auth) . | ||
| (\r -> swaggerSchemaUIServer (documentation args.auth) :<|> server (serverArgs args caps r eb whitelist)) | ||
| (\r -> swaggerSchemaUIServer (documentation args.auth) | ||
| :<|> server (serverArgs args caps r eb whitelist addressRotation)) |
There was a problem hiding this comment.
The call to startTransactionsMonitor now includes args.minAmountForAddressAssessment as an argument. Ensure that this value is passed correctly and used appropriately within the function. Also, the creation of addressRotation using newMVar emptyAddressRotation is a new addition. Make sure that addressRotation is used correctly in the server.
There was a problem hiding this comment.
@coderabbitai what do you mean by "used correctly in the server" ?
There was a problem hiding this comment.
@bogdan-manole By "used correctly in the server", I mean that the addressRotation variable should be used in a way that is thread-safe, consistent with its intended purpose, and does not introduce any potential issues such as data races or deadlocks.
The addressRotation variable is created using newMVar emptyAddressRotation, which suggests it's intended to be used as a mutable shared state in a multi-threaded environment. Therefore, when this variable is accessed or modified, it should be done so within an MVar operation (like modifyMVar_, readMVar, etc.) to ensure atomicity and avoid potential data races.
Moreover, the usage of addressRotation should align with its intended purpose. If addressRotation is meant to keep track of address rotation status, then it should be updated appropriately whenever an address rotation occurs, and its value should be checked before making decisions based on the rotation status.
Lastly, care should be taken to avoid potential deadlocks. For instance, if there are multiple MVars and a thread needs to acquire more than one, they should always be acquired in the same order by all threads.
In summary, "used correctly" means that addressRotation should be used in a thread-safe manner, its value should accurately represent the current state of address rotation, and its usage should not introduce potential concurrency issues.
| @@ -45,6 +45,7 @@ import IOHK.Certification.SignatureVerification | |||
| (COSEKey,COSESign1, decodeHex,encodeHex) | |||
| import Data.Char (isAlphaNum) | |||
| import Text.Regex | |||
| import Data.Int | |||
There was a problem hiding this comment.
The import of Data.Int has been added, but it's not clear why this is needed as there are no changes in the hunk that seem to require it. Please ensure that this import is necessary.
- import Data.Int| import Data.Int | ||
| import Data.Bifunctor | ||
|
|
||
| import Data.Functor | ||
|
|
||
| import qualified Data.Map as Map |
There was a problem hiding this comment.
The newly imported modules Data.Int, Data.Bifunctor, Data.Functor and Data.Map as Map are not used in the provided context. If they are not used elsewhere in the code, consider removing these imports to maintain clean and efficient code.
- import Data.Int
- import Data.Bifunctor
- import Data.Functor
- import qualified Data.Map as Map| @@ -45,6 +48,7 @@ data InitializingField | |||
| data SynchronizerSelector f where | |||
| InitializingSynchronizer :: SynchronizerSelector InitializingField | |||
| InjectTxBroadcaster :: forall f . !(TxBroadcasterSelector f) -> SynchronizerSelector f | |||
| InjectProfileWalletSync :: forall f . !(ProfileWalletSyncSelector f) -> SynchronizerSelector f | |||
There was a problem hiding this comment.
A new action InjectProfileWalletSync is added to the SynchronizerSelector data type. Make sure that this new action is handled correctly in all functions that pattern match on SynchronizerSelector.
| DB.withDb $ forM_ transactions storeTransaction | ||
| where |
There was a problem hiding this comment.
The filtering of incoming transactions has been removed, which means that both incoming and outgoing transactions will be stored in the database. If this is not the intended behavior, consider adding back the filter.
- DB.withDb $ forM_ incomingTransactions storeTransaction
+ DB.withDb $ forM_ transactions storeTransactionThere was a problem hiding this comment.
outgoing transaction are to be taken in consideration as well, because the system now will broadcast by itself transactions to solve overlapping addresses for different profiles
| @@ -203,7 +220,7 @@ | |||
| certifyRuns' [] _ = return () | |||
| certifyRuns' (run:rs) creditsAvailable = do | |||
| -- calculate the cost of the run | |||
| let cost = run.certificationPrice | |||
| let cost = fromIntegral run.certificationPrice | |||
There was a problem hiding this comment.
The cost of the run is now converted to an integer. This could potentially lead to loss of precision if the price was originally a floating-point number. Make sure this conversion is appropriate and does not introduce any inaccuracies.
| , getProfileWalletAddress = \(profileId,_) -> withEvent eb GetProfileWalletAddress \ev -> do | ||
| addField ev profileId | ||
| -- first check the db | ||
| walletM <- (id <=< fmap snd) <$> DB.withDb ( DB.getProfileWallet profileId ) | ||
|
|
||
| -- second, if there is nothing in the db try to get | ||
| case walletM of | ||
| Just (DB.ProfileWallet _ address status _) -> | ||
| pure $ Just (status, address) | ||
| Nothing -> do | ||
| resp <- Wallet.getWalletAddresses serverWalletArgs (Just Wallet.Unused) | ||
| case resp of | ||
| Right unusedAddressesInfo -> do | ||
| let unusedAddresses = fmap (PW.WalletAddress . (.addressId)) unusedAddressesInfo | ||
| (walletAddressM,newRotation) <- liftIO (takeMVar serverAddressRotation) | ||
| <&> getTemporarilyWalletAddress unusedAddresses profileId | ||
| liftIO $ putMVar serverAddressRotation newRotation | ||
| pure $ fmap ((DB.Overlapping,) . unWalletAddress) walletAddressM | ||
| Left err -> withEvent eb InternalError $ \ev' -> do | ||
| addField ev' (show err) | ||
| throwError $ err500 {errBody = LSB.pack $ show resp} | ||
|
|
There was a problem hiding this comment.
A new endpoint getProfileWalletAddress has been added. This endpoint retrieves the wallet address associated with a profile. It first checks the database for the wallet address, and if it doesn't find one, it tries to get an unused address from the wallet service. The logic seems correct, but please ensure that error handling is properly implemented and that the wallet service's API is used correctly.
There was a problem hiding this comment.
this should be deleted
bogdan-manole
force-pushed
the
feat/dedicated-payment-addresses
branch
from
531beae
to
4ab2548
Compare
There was a problem hiding this comment.
Actionable comments generated: 3
Files ignored due to filter (1)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/example.json
Files selected for processing (16)
- client/Main.hs (3 hunks)
- dapps-certification-persistence/dapps-certification-persistence.cabal (1 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence.hs (4 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/API.hs (5 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/Structure.hs (5 hunks)
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1 hunks)
- plutus-certification.cabal (2 hunks)
- server/Main.hs (8 hunks)
- src/Plutus/Certification/API/Routes.hs (5 hunks)
- src/Plutus/Certification/API/Swagger.hs (1 hunks)
- src/Plutus/Certification/CertificationBroadcaster.hs (2 hunks)
- src/Plutus/Certification/ProfileWallet.hs (1 hunks)
- src/Plutus/Certification/Server/Instance.hs (9 hunks)
- src/Plutus/Certification/Server/Internal.hs (4 hunks)
- src/Plutus/Certification/Synchronizer.hs (10 hunks)
- src/Plutus/Certification/WalletClient.hs (5 hunks)
Files skipped from review due to trivial changes (7)
- dapps-certification-persistence/dapps-certification-persistence.cabal
- dapps-certification-persistence/src/IOHK/Certification/Persistence/Structure.hs
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs
- plutus-certification.cabal
- src/Plutus/Certification/API/Swagger.hs
- src/Plutus/Certification/CertificationBroadcaster.hs
- src/Plutus/Certification/ProfileWallet.hs (diff too large)
LGTM comments (Suppressed): 12
src/Plutus/Certification/API/Routes.hs (1)
- 35-40: The import of
Data.Inthas been moved to line 50. This change doesn't seem to have any impact on the functionality, but it's worth noting.server/Main.hs (1)
- 159-165: The parser for the new command-line argument
min-amount-for-address-assessmenthas been added correctly. It uses theautofunction fromoptparse-applicativeto automatically parse the argument as aWord64. The default value is set tooneAda.src/Plutus/Certification/Server/Internal.hs (1)
- 125-125: A new case for rendering the
GetProfileWalletAddressserver event selector has been added. This should be fine as long as therenderProfileIdfunction correctly renders aProfileId.src/Plutus/Certification/Server/Instance.hs (3)
115-120: The
createRunfunction now uses the renamedserverGithubTokeninstead ofgithubToken. This change seems consistent with the renaming in theServerArgsdata type.236-236: The
ghAccessTokenM'now uses the renamedserverGithubTokeninstead ofgithubToken. This change seems consistent with the renaming in theServerArgsdata type.304-308: An extra line break has been added here. It doesn't affect the functionality of the code but might be worth noting for consistency.
src/Plutus/Certification/WalletClient.hs (4)
40-44: The order of imports has been changed. This is not an issue as long as all necessary modules are imported correctly.
94-114: A new API endpoint
AddressesAPIhas been added to the existingAPItype. TheAddressStatedata type and its instances have also been introduced. These changes seem to be in line with the PR's goal of adding functionality related to profile wallets.157-166: The
broadcastTransactionfunction now includes an optional destination address parameter. This change seems to be consistent with the PR's goal of introducing dedicated payment addresses for profiles.176-194: New data type
WalletAddressInfoand a new functiongetWalletAddresseshave been introduced. These changes are in line with the PR's goal of adding functionality related to profile wallets.src/Plutus/Certification/Synchronizer.hs (2)
78-78: The function
renderSynchronizerSelectorhas been extended to handle the newInjectProfileWalletSyncselector. This looks good as long asrenderProfileWalletSyncSelectoris correctly implemented.105-106: The filtering of incoming transactions has been removed, which means that both incoming and outgoing transactions will be stored in the database. As per the conversation chain, this is intended behavior.
Additional comments (Suppressed): 36
src/Plutus/Certification/API/Routes.hs (3)
2-2: The
OverloadedListslanguage extension has been removed. Ensure that this does not affect any existing code that relies on this feature.142-147: A new route
GetProfileWalletAddressRoutehas been added for getting the wallet address of a profile. Make sure that this route is correctly implemented in the server's main function and that it properly handles potential errors (e.g., if the profile does not exist or if the wallet address cannot be retrieved).306-306: The new route
getProfileWalletAddresshas been added to the API. Ensure that all necessary authentication and authorization checks are performed when this route is accessed.dapps-certification-persistence/src/IOHK/Certification/Persistence.hs (4)
23-24: The new data types
ProfileWalletandWalletAddressStatusare being exported. Ensure that these types are defined correctly in their respective modules and that they are used appropriately throughout the codebase.42-44: The new data types
MinimalTransactionandMinimalTransactionEntryare being exported. Make sure these types are defined correctly in their respective modules and that they are used appropriately throughout the codebase.66-67: The function
getProfileBalancehas been moved. Ensure that all calls to this function have been updated to reflect its new location.78-81: New functions
getAllTransactions,getProfileWallets,getProfileWallet, andupsertProfileWalletare being exported. Verify that these functions are implemented correctly and used appropriately throughout the codebase.client/Main.hs (3)
226-241: The new commands
get-wallet-addressandget-balancehave been added to thecurrentProfileParser. Ensure that these commands are documented in the user manual or help documentation for the command-line tool. Also, verify that appropriate error handling is in place for these commands in case of any failures.403-404: The
ProfileCommanddata type has been extended with two new constructors:GetProfileWalletAddressandGetProfileBalance. Make sure all pattern matching onProfileCommandthroughout the codebase has been updated to handle these new cases.603-606: The main function has been updated to handle the new
GetProfileWalletAddressandGetProfileBalancecommands. Ensure that the functionsc.getProfileWalletAddressandc.getProfileBalanceare properly implemented and tested.server/Main.hs (4)
91-91: A new field
minAmountForAddressAssessmenthas been added to theArgsdata type. Ensure that all instances whereArgsis used have been updated to include this new field.434-434: A new mutable variable
addressRotationis introduced usingnewMVar. Make sure that all accesses to this variable are properly synchronized to avoid potential data races.443-443: The server function now includes an additional parameter
addressRotation. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.484-487: The
startTransactionsMonitorfunction now accepts an additional parameterminAmountForAddressAssessment. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.src/Plutus/Certification/Server/Internal.hs (2)
3-3: The
TupleSectionslanguage extension has been added. Ensure that this change is necessary and consistent with the rest of the codebase.107-108: Two new server event selectors have been added:
GetProfileWalletAddressandInternalError. Make sure these are handled appropriately in the server's main function and that they don't introduce any breaking changes.dapps-certification-persistence/src/IOHK/Certification/Persistence/API.hs (4)
144-157: The logic for calculating
creditsAvailablehas been changed. Previously, it was calculated based on the sum of all amounts coming from a profile's address. Now, it's calculated based on the credits available in the profile's wallet. This change seems logical considering the introduction of profile wallets. However, ensure that thegetProfileWalletfunction andprofileWalletCreditsfield are correctly implemented and tested.216-248: New functions have been added to handle profile wallets. These include
getProfileWallet,getProfileWallets, andupsertProfileWallet. The implementation looks correct, but make sure these new functions are covered with unit tests to verify their correctness.292-297: The function
getAllAmountsForAddresshas been removed. Ensure that this function is not used anywhere else in the codebase or its removal could cause breaking changes.553-597: A new function
getAllTransactionshas been added along with the data typesMinimalTransactionandMinimalTransactionEntry. The function retrieves all transactions and their entries, optionally filtering only output entries. The implementation seems correct, but ensure that this function is covered with unit tests to verify its correctness.src/Plutus/Certification/Server/Instance.hs (6)
2-2: The addition of the
TupleSectionslanguage extension is not used in any of the provided code hunks. Please ensure that it's necessary and used somewhere else in the code.38-38: The import statement for
Plutus.Certification.Server.Internalhas been changed to use a qualified import asI. Ensure that all references to this module in the codebase have been updated accordingly.49-58: New imports have been added, including
Plutus.Certification.ProfileWalletasPWandData.Functor. Ensure these new imports are used appropriately throughout the codebase.79-87: The
ServerArgsdata type has been updated with renamed fields (serverGithubToken,serverJWTConfig,serverValidateSubscriptions,serverAdaUsdPrice) and a new fieldserverAddressRotation. Make sure all instances ofServerArgsin the codebase have been updated to match the new structure.321-342: A new function
getProfileWalletAddresshas been added. This function retrieves the wallet address for a given profile ID. It first checks the database for the wallet address, and if not found, it tries to get an unused address from the wallet service. The function also handles potential errors from the wallet service. This is a significant addition and should be thoroughly tested.345-361: The helper functions
handleException,getAdaUsdPrice',validateFeature,fromClientResponse, andserverErrorFromClientErrorhave been moved within thewhereclause of theserverfunction. Ensure that these changes do not affect their scope and usage in the rest of the code.src/Plutus/Certification/WalletClient.hs (2)
1-26: The new hunk introduces a few more language extensions and exports additional types (
TxId,getWalletAddresses,AddressState,WalletAddressInfo). Ensure that these new additions are used appropriately throughout the codebase.145-146: The
mkClientfunction signature has been updated to include thegetWalletAddressesfunction. Make sure all calls tomkClienthave been updated accordingly.src/Plutus/Certification/Synchronizer.hs (8)
29-29: The module
Plutus.Certification.TransactionBroadcasterhas been renamed toPlutus.Certification.CertificationBroadcaster. Ensure that all references to this module in the codebase have been updated accordingly.41-44: The import of
Data.IORefhas been moved andPlutus.Certification.ProfileWalletandData.Word (Word64)have been added. Ensure these new imports are used appropriately in the subsequent code.54-54: A new selector
InjectProfileWalletSynchas been added to theSynchronizerSelectordata type. Make sure it's handled correctly in all functions that pattern match onSynchronizerSelector.116-126: The calculation of
DB.wtxAmountnow takes into account the direction of the transaction. This is a significant change and should be thoroughly tested to ensure it doesn't introduce any bugs or inconsistencies.160-161: There are TODO comments about removing conversions after merging with
feat/subscription. Ensure that these conversions are indeed removed after the merge.166-184: The function
monitorWalletTransactionsnow accepts two additional parameters:minAssignmentAmountandrefAssignments. It also includes a call toresyncWallets. Ensure that all calls to this function throughout the codebase have been updated to match the new signature and thatresyncWalletsis correctly implemented.244-244: The cost of the run is now calculated using
fromIntegral. This could potentially introduce rounding errors. Make sure this change doesn't affect the accuracy of the cost calculation.261-275: The function
startTransactionsMonitornow accepts an additional parameterminAssignmentAmountand includes a call tomonitorWalletTransactionswith additional arguments. Ensure that all calls to this function throughout the codebase have been updated to match the new signature and thatmonitorWalletTransactionsis correctly implemented.
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
server/Main.hs
Outdated
| import Plutus.Certification.ProfileWallet | ||
| import Paths_plutus_certification qualified as Package | ||
| import IOHK.Certification.Persistence qualified as DB | ||
| import Data.HashSet as HashSet | ||
| import Data.Word | ||
|
|
||
| import qualified Data.ByteString.Lazy.Char8 as LBS | ||
| import qualified Data.Text as Text | ||
| import System.Environment (lookupEnv) | ||
| import Crypto.Random | ||
|
|
||
|
|
||
| oneAda :: Word64 | ||
| oneAda = 1000000 |
There was a problem hiding this comment.
The import statements are not properly aligned. This is not a major issue but it can affect readability and maintainability of the code. Please align them properly.
import Plutus.Certification.ProfileWallet
import Paths_plutus_certification qualified as Package
import IOHK.Certification.Persistence qualified as DB
import Data.HashSet as HashSet
import Data.Word
import qualified Data.ByteString.Lazy.Char8 as LBS
import qualified Data.Text as Text
import System.Environment (lookupEnv)
import Crypto.Random
import Plutus.Certification.ProfileWallet
import Paths_plutus_certification qualified as Package
import IOHK.Certification.Persistence qualified as DB
import Data.HashSet as HashSet
import Data.Word
import qualified Data.ByteString.Lazy.Char8 as LBS
import qualified Data.Text as Text
import System.Environment (lookupEnv)
import Crypto.Random| renderServerEventSelector InternalError = | ||
| ("internal-error", ("something-went-wrong",) . toJSON ) |
There was a problem hiding this comment.
A new case for rendering the InternalError server event selector has been added. This seems to always render "something-went-wrong" as part of the JSON, which might not provide enough information about the error. Consider including more detailed error information.
- ("internal-error", ("something-went-wrong",) . toJSON )
+ ("internal-error", \case
+ (SomeError err) -> ("error", toJSON err)
+ _ -> ("unknown-error", "An unknown error occurred"))
bogdan-manole
force-pushed
the
feat/dedicated-payment-addresses
branch
from
4ab2548
to
9832e14
Compare
There was a problem hiding this comment.
Actionable comments generated: 3
Files ignored due to filter (1)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/example.json
Files selected for processing (16)
- client/Main.hs (3 hunks)
- dapps-certification-persistence/dapps-certification-persistence.cabal (1 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence.hs (4 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/API.hs (6 hunks)
- dapps-certification-persistence/src/IOHK/Certification/Persistence/Structure.hs (5 hunks)
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1 hunks)
- plutus-certification.cabal (2 hunks)
- server/Main.hs (8 hunks)
- src/Plutus/Certification/API/Routes.hs (5 hunks)
- src/Plutus/Certification/API/Swagger.hs (1 hunks)
- src/Plutus/Certification/CertificationBroadcaster.hs (2 hunks)
- src/Plutus/Certification/ProfileWallet.hs (1 hunks)
- src/Plutus/Certification/Server/Instance.hs (10 hunks)
- src/Plutus/Certification/Server/Internal.hs (4 hunks)
- src/Plutus/Certification/Synchronizer.hs (11 hunks)
- src/Plutus/Certification/WalletClient.hs (5 hunks)
Files skipped from review due to trivial changes (7)
- dapps-certification-persistence/dapps-certification-persistence.cabal
- dapps-certification-persistence/src/IOHK/Certification/Persistence/Structure.hs
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs
- plutus-certification.cabal
- server/Main.hs
- src/Plutus/Certification/Server/Internal.hs
- src/Plutus/Certification/ProfileWallet.hs (diff too large)
LGTM comments (Suppressed): 2
src/Plutus/Certification/WalletClient.hs (1)
- 40-44: The order of imports has been changed. This is not an issue, but it's good to keep the order consistent across the project for better readability.
src/Plutus/Certification/Server/Instance.hs (1)
- 311-315: Empty lines have been added before and after the
getAdaUsdPricefunction. This doesn't affect the functionality of the code, but it improves readability by separating different sections of the code.
Additional comments (Suppressed): 41
src/Plutus/Certification/API/Swagger.hs (1)
- 54-54: The new route
GetProfileWalletAddressRoutehas been added. Ensure that the corresponding handler for this route is implemented and that it correctly retrieves the wallet address of a profile. Also, verify that appropriate access controls are in place for this route to prevent unauthorized access to sensitive data.src/Plutus/Certification/CertificationBroadcaster.hs (2)
14-17: The module name has been changed from
TransactionBroadcastertoCertificationBroadcaster. Ensure that all imports and usages of this module throughout the codebase have been updated to match the new name.79-81: The
broadcastTransactionfunction now accepts an additional argumentNothing. Verify if this change is consistent with the updated function signature and doesn't break existing functionality. Also, ensure that the magic number1304is correctly used here or consider replacing it with a well-named constant for better readability.dapps-certification-persistence/src/IOHK/Certification/Persistence.hs (4)
27-28: The new data types
ProfileWalletandWalletAddressStatushave been added to the exports. Ensure that these types are defined correctly in their respective modules and that they are used appropriately throughout the codebase.46-48: The new data types
MinimalTransactionandMinimalTransactionEntryhave been added to the exports. Make sure these types are defined correctly in their respective modules and that they are used appropriately throughout the codebase.70-72: The function
getProfileBalancehas been moved, and the functiongetAllAmountsForAddresshas been removed. Verify that this change does not break any existing functionality and that all calls to these functions have been updated accordingly.82-85: New functions
getAllTransactions,getProfileWallets,getProfileWallet, andupsertProfileWallethave been added to the exports. Ensure that these functions are implemented correctly and that they are used appropriately throughout the codebase.src/Plutus/Certification/API/Routes.hs (3)
2-2: The
OverloadedListslanguage extension has been removed. Ensure that this does not affect any existing code that relies on this extension.142-147: A new route
GetProfileWalletAddressRoutehas been added to retrieve the wallet address of a profile. Make sure that the necessary authentication and authorization checks are implemented for this route to prevent unauthorized access to sensitive data.306-306: The function
getProfileWalletAddresshas been added to the API. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.client/Main.hs (3)
226-241: The new commands
get-wallet-addressandget-balancehave been added to thecurrentProfileParser. Ensure that these commands are documented in the user guide or help documentation for the command-line tool. Also, verify that all calls to this parser throughout the codebase have been updated to handle these new commands.403-404: The
ProfileCommanddata type has been extended with two new constructors:GetProfileWalletAddressandGetProfileBalance. Make sure that all pattern matches onProfileCommandvalues in the codebase have been updated to handle these new constructors.603-606: The handling of
ProfileCommandvalues in the main function has been updated to include the newGetProfileWalletAddressandGetProfileBalancecommands. The implementation looks correct as it uses thewithAuthhelper function to authenticate the request before calling the appropriate method on the API client. However, ensure that error handling is consistent with other commands.dapps-certification-persistence/src/IOHK/Certification/Persistence/API.hs (5)
68-69: The variable name has been changed from
selecttorecord. Ensure that this change does not affect other parts of the code whereselectmight have been used.145-152: The logic for calculating
creditsAvailablehas been updated to includewalletIncomingCreditsinstead ofamountsFromAddress. Make sure that this change is reflected everywhere in the codebase wherecreditsAvailableis used.217-249: New functions have been added to handle profile wallets. These include
getProfileWalletQ,getProfileWallet,toTuple,getProfileWalletsQ,getProfileWallets, andupsertProfileWallet. Ensure that these new functions are correctly integrated with the rest of the codebase and that all necessary updates have been made.293-298: The function
getAllAmountsForAddresshas been removed. Verify that this function is not being used anywhere else in the codebase, or if it is, that appropriate changes have been made.554-598: New data types
MinimalTransactionandMinimalTransactionEntryhave been introduced along with a new functiongetAllTransactions. Ensure that these new data types and function are correctly integrated with the rest of the codebase and that all necessary updates have been made.src/Plutus/Certification/WalletClient.hs (5)
1-26: The new hunk introduces a few additional exports such as
TxId,getWalletAddresses,AddressState, andWalletAddressInfo. Ensure that these new exports are used appropriately in the rest of the codebase.94-114: The API type has been extended with
AddressesAPIwhich includes a new endpoint for getting wallet addresses based on their state (used or unused). The implementation seems correct, but ensure that this new endpoint is correctly integrated into the server and that the server can handle requests to this endpoint properly.145-146: The
mkClientfunction signature has been updated to include a new function for getting wallet addresses. Make sure all calls tomkClienthave been updated accordingly.157-166: The
broadcastTransactionfunction now accepts an optional destination address parameter. If no destination address is provided, the wallet address from theWalletArgsis used. This change seems logical and should provide more flexibility when broadcasting transactions. However, make sure that all calls tobroadcastTransactionhave been updated to match the new function signature.176-194: New data type
WalletAddressInfoand a new functiongetWalletAddresseshave been introduced. The function retrieves wallet addresses based on their state (used or unused). The implementation looks correct, but ensure that this function is used correctly in the rest of the codebase.src/Plutus/Certification/Synchronizer.hs (10)
29-29: The module
Plutus.Certification.TransactionBroadcasterhas been renamed toPlutus.Certification.CertificationBroadcaster. Ensure that all references to this module in the codebase have been updated accordingly.42-48: New imports have been added. Ensure that these new imports do not introduce any unused dependencies or potential conflicts with existing code.
59-59: A new selector
InjectProfileWalletSynchas been introduced. Make sure it is used correctly throughout the codebase and its implementation is correct.84-84: Ensure that the function
renderProfileWalletSyncSelectoris defined and implemented correctly elsewhere in the codebase.121-131: The transaction amount is now set based on the direction of the transaction. This is a significant change in how transactions are handled and could potentially impact other parts of the system. Ensure that this change is reflected in all parts of the system that deal with transaction amounts.
165-166: There's a TODO comment about removing conversion after merging with
feat/subscription. Ensure that this is tracked and addressed when the merge happens.171-189: The function
monitorWalletTransactionshas been significantly modified. It now includes synchronization of wallets and transaction amount conversion based on the direction of the transaction. Ensure that these changes are reflected in all calls to this function and that they do not introduce any unintended side effects.249-249: The cost of the run is now converted to an integer. Ensure that this does not result in loss of precision or incorrect calculations elsewhere in the system.
267-285: The function
startTransactionsMonitorandstartTransactionsMonitor'now include an additional parameterminAssignmentAmount. Ensure that all calls to these functions have been updated to include this new parameter.299-304: The
doWorkfunction now includes a call tomonitorWalletTransactionswith an additional parameterref. Ensure that this change is reflected in all calls todoWorkand that it does not introduce any unintended side effects.src/Plutus/Certification/Server/Instance.hs (8)
2-2: The addition of the
TupleSectionslanguage extension is not used in any of the provided code hunks. Please ensure that it's necessary and used somewhere else in the code.38-38: The import statement for
Plutus.Certification.Server.Internalhas been changed to use a qualified import with an aliasI. Ensure that all references to this module in the codebase have been updated accordingly.49-50: The new import
import Plutus.Certification.ProfileWallet as PWis introduced, but there's no usage of it in the provided hunks. Make sure it's used elsewhere in the code. Also, the importimport Data.Functorseems unnecessary as its functions are usually available by default. If it's not used, consider removing it.59-59: The import
import Control.Concurrent (MVar, takeMVar, putMVar)is added. This indicates that mutable state might be used in the application. Mutable state can lead to data races if not handled properly. Ensure that all operations onMVars are safe and won't cause any race conditions.80-89: The
ServerArgsdata type has been modified. The fieldserverGithubTokenhas been renamed fromgithubToken, and a new fieldserverAddressRotation :: MVar AddressRotationhas been added. Ensure that all usages of these fields throughout the codebase have been updated to match the new names and types.118-123: The function call
withEvent eb I.WalletAddress . const $ pure serverWalletArgs.walletAddresshas been modified to useI.WalletAddressinstead ofWalletAddress. Ensure that this change is consistent with the rest of the codebase.242-242: The variable
ghAccessTokenM'now usesserverGithubTokeninstead ofgithubToken. Ensure that this change is consistent with the rest of the codebase.327-370: A new function
getProfileWalletAddresshas been added. It retrieves the wallet address of a profile. The logic seems correct: it first checks the database for the wallet address, and if it doesn't find one, it tries to get an unused address from the wallet service. However, the handling of theMVarserverAddressRotationcould potentially lead to data races if not done correctly. Ensure that all operations onserverAddressRotationare thread-safe.
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| import IOHK.Certification.Interface | ||
| import Data.Time | ||
| import Data.Proxy | ||
| import Data.Int | ||
| import Text.Read hiding (String) | ||
| import Plutus.Certification.WalletClient | ||
| import Control.Lens hiding ((.=)) |
There was a problem hiding this comment.
The import of Data.Int has been moved from line 36 to line 50. This change seems unnecessary and could potentially cause confusion in the future. It's generally a good practice to keep imports grouped and ordered logically, for example by the package or functionality they provide.
| import Data.Fixed | ||
| import Data.Int | ||
| import Data.Bifunctor | ||
|
|
||
| import Data.Fixed | ||
| import Data.Functor | ||
|
|
||
| import qualified Data.Map as Map |
There was a problem hiding this comment.
There are unnecessary spaces and an extra import statement for Data.Functor which is not used in the code. This can be removed to keep the code clean.
import Data.Fixed
import Data.Int
import Data.Bifunctor
import Data.Functor
import qualified Data.Map as Map
import Data.Fixed
import Data.Int
import Data.Bifunctor
import qualified Data.Map as Map| @@ -103,10 +107,8 @@ walletTxStatusToDbStatus Submitted = DB.Submitted | |||
|
|
|||
| synchronizeDbTransactions :: (MonadIO m, MonadMask m,MonadReader env m,HasDb env) => [WalletTransaction] -> m () | |||
| synchronizeDbTransactions transactions = do | |||
| -- filter out the outgoing transactions and sync them with the database | |||
| withDb $ forM_ incomingTransactions storeTransaction | |||
| withDb $ forM_ transactions storeTransaction | |||
There was a problem hiding this comment.
The logic for storing transactions has changed from only storing incoming transactions to storing all transactions. This could potentially lead to a significant increase in the amount of data stored. Consider if this change is necessary and if the system can handle the increased data load.
There was a problem hiding this comment.
Actionable comments generated: 1
Files selected for processing (6)
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1 hunks)
- plutus-certification.cabal (3 hunks)
- src/Plutus/Certification/ProfileWallet.hs (1 hunks)
- src/Plutus/Certification/Synchronizer.hs (10 hunks)
- test/ProfileWalletSpec.hs (1 hunks)
- test/Spec.hs (1 hunks)
Files skipped from review due to trivial changes (3)
- test/ProfileWalletSpec.hs
- test/Spec.hs
- src/Plutus/Certification/ProfileWallet.hs (diff too large)
Additional comments (Suppressed): 13
dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1)
- 11-12: The function
bech32AddressHashand the typeHashErrorhave been added to the exports of this module. Ensure that these are intended for public use and that their usage is consistent with the rest of the codebase.plutus-certification.cabal (3)
75-75: The new module
Plutus.Certification.ProfileWallethas been added to the exposed modules of the library. Ensure that this module is intended to be exposed and that it does not expose any sensitive implementation details or functionality that should remain internal.84-84: The module
Plutus.Certification.TransactionBroadcasterhas been renamed toPlutus.Certification.CertificationBroadcaster. Make sure all references to this module in the codebase have been updated accordingly.157-157: A new test suite
ProfileWalletSpechas been added. Ensure that this test suite covers all the critical functionalities and edge cases of theProfileWalletmodule.src/Plutus/Certification/Synchronizer.hs (9)
29-49: The module
Plutus.Certification.TransactionBroadcasterhas been renamed toPlutus.Certification.CertificationBroadcaster. Ensure that all references to this module throughout the codebase have been updated accordingly.60-60: A new selector
InjectProfileWalletSynchas been added. Make sure it is handled in all functions that pattern match onSynchronizerSelector.85-85: Ensure that
renderProfileWalletSyncSelectorfunction exists and correctly renders theProfileWalletSyncSelector.122-132: The transaction amount is now being set based on the direction of the transaction. This is a significant change in logic and should be thoroughly tested to ensure it behaves as expected.
166-167: There are TODO comments about removing conversion after merging with
feat/subscription. Ensure that these conversions are indeed removed after the merge.174-204: The
monitorWalletTransactionsfunction now accepts two additional parameters:minAssignmentAmountandrefAssignments. Ensure that all calls to this function throughout the codebase have been updated to pass these new arguments.256-257: The
certificationPriceis now being converted to an integral type. Ensure that this does not lead to loss of precision or incorrect results due to rounding.274-292: The
startTransactionsMonitorandstartTransactionsMonitor'functions now accept an additional parameterminAssignmentAmount. Ensure that all calls to these functions throughout the codebase have been updated to pass this new argument.306-311: The
monitorWalletTransactionsfunction is now being called with an additional argumentref. Ensure that this change is reflected wherever this function is used.
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| withDb $ forM_ transactions storeTransaction | ||
| where |
There was a problem hiding this comment.
The filtering of incoming transactions before storing them in the database has been removed. This means that now all transactions, regardless of their direction, will be stored. If this is intentional and you want to store all transactions, then it's fine. Otherwise, consider reintroducing the filter.
- withDb $ forM_ incomingTransactions storeTransaction
+ withDb $ forM_ transactions storeTransaction
bogdan-manole
force-pushed
the
feat/dedicated-payment-addresses
branch
from
10f8898
to
b9857cf
Compare
There was a problem hiding this comment.
Actionable comments generated: 2
Files selected for processing (10)
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1 hunks)
- plutus-certification.cabal (3 hunks)
- src/Plutus/Certification/CertificationBroadcaster.hs (4 hunks)
- src/Plutus/Certification/ProfileWallet.hs (1 hunks)
- src/Plutus/Certification/Server/Instance.hs (10 hunks)
- src/Plutus/Certification/Synchronizer.hs (11 hunks)
- src/Plutus/Certification/WalletClient.hs (5 hunks)
- test/ProfileWallet/Data.hs (1 hunks)
- test/ProfileWalletSpec.hs (1 hunks)
- test/Spec.hs (1 hunks)
Files skipped from review due to trivial changes (3)
- test/ProfileWallet/Data.hs
- test/Spec.hs
- src/Plutus/Certification/ProfileWallet.hs (diff too large)
LGTM comments (Suppressed): 4
plutus-certification.cabal (1)
- 152-166: A new test suite
plutus-certification-testhas been added. This is a good practice for ensuring the correctness of the code. However, make sure that the tests cover all important aspects of the new functionality and that they pass before merging the PR.src/Plutus/Certification/WalletClient.hs (1)
- 173-179: No significant issues detected in these lines. The
getTransactionList'function seems to be correctly implemented.src/Plutus/Certification/Server/Instance.hs (2)
311-315: An empty line has been added before the
getAdaUsdPricefunction. This change is purely stylistic and does not affect the logic of the program.442-442: An empty line has been added before the
getRunAndSyncfunction. This change is purely stylistic and does not affect the logic of the program.
Additional comments (Suppressed): 37
src/Plutus/Certification/CertificationBroadcaster.hs (4)
14-14: The module name has been changed from
TransactionBroadcastertoCertificationBroadcaster. Ensure that all imports and usages of this module throughout the codebase have been updated to match the new name.33-33: The import statement has been changed from
WalletArgstoWalletClient. Make sure that theWalletClientis correctly implemented and used in place ofWalletArgs.59-62: The function signature for
createL1Certificationhas been modified. The second parameter has been changed fromWalletArgstoWalletClient. Verify that all calls to this function throughout the codebase have been updated to match the new signature.80-80: The function call
broadcastTransactionhas been replaced withbroadcastTx. Ensure thatbroadcastTxis correctly implemented and provides the same functionality asbroadcastTransaction.dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1)
- 11-12: The addition of
bech32AddressHashandHashError(..)to the exports list suggests that these functions or types have been newly created or made public. Ensure that their implementations are correct, secure, and properly tested. Also, verify if they are used correctly wherever they are imported.plutus-certification.cabal (2)
75-75: The new module
Plutus.Certification.ProfileWallethas been added to the exposed modules of the library. Ensure that this module is intended to be exposed and that it does not expose any sensitive implementation details or security risks.84-84: The module
Plutus.Certification.TransactionBroadcasterhas been renamed toPlutus.Certification.CertificationBroadcaster. Make sure all references to this module in the codebase have been updated accordingly.src/Plutus/Certification/WalletClient.hs (8)
1-31: The new code introduces several new types and functions in the export list of the module. Ensure that these new exports are used appropriately throughout the codebase and do not conflict with existing names.
45-49: The order of imports has been changed, and
Data.Maybehas been added. Ensure that this does not cause any conflicts or shadowing issues in the rest of the code.99-112: The API type has been extended to include an
AddressesAPI. This is a significant change and should be thoroughly tested to ensure it works as expected and doesn't introduce any breaking changes.150-151: The
mkClientfunction signature has been modified to include a new function for getting wallet addresses. Make sure all calls tomkClienthave been updated accordingly.160-172: The
broadcastTransaction'function now includes an optional destination address parameter. This is a significant change and should be thoroughly tested to ensure it works as expected and doesn't introduce any breaking changes.181-191: A new data type
WalletAddressInfohas been introduced along with its JSON parsing instance. Ensure that the JSON keys ("derivation_path", "id", "state") match the actual keys in the response from the server.192-199: The
getWalletAddresses'function retrieves a list of wallet addresses from the API. Ensure that this function is used correctly throughout the codebase and handles potential errors properly.201-226: The
WalletClientdata type and related type aliases have been introduced. These seem to be correctly implemented, but they should be thoroughly tested to ensure they work as expected.src/Plutus/Certification/Server/Instance.hs (8)
2-2: The addition of the
TupleSectionslanguage extension is not used anywhere in the provided code hunks. If it's not used elsewhere in the file, consider removing it to avoid unnecessary complexity.38-38: The import statement for
Plutus.Certification.Server.Internalhas been changed to use a qualified import with an aliasI. Ensure that all references to this module in the codebase have been updated accordingly.49-50: The new import statement for
Plutus.Certification.ProfileWalletasPWandData.Functorare introduced. Make sure these modules are used in the rest of the code.59-59: The import statement for
Control.ConcurrentintroducesMVar,takeMVar, andputMVar. These functions are used for mutable state in concurrent programming. Ensure that their usage follows best practices to avoid potential data races or deadlocks.80-89: The
ServerArgsdata type has been modified. The fieldgithubTokenhas been renamed toserverGithubToken,validateSubscriptionstoserverValidateSubscriptions,adaUsdPricetoserverAdaUsdPrice, and a new fieldserverAddressRotationof typeMVar AddressRotationhas been added. Ensure that all references to these fields in the codebase have been updated accordingly. Also, ensure that the newserverAddressRotationfield is properly initialized and used correctly to avoid potential issues with concurrency.118-123: The
walletAddressandcreateRunfunctions have been updated. ThewalletAddressfunction now usesI.WalletAddressinstead ofWalletAddressfor event logging. ThecreateRunfunction now usesserverGithubTokeninstead ofgithubToken. Ensure that these changes do not break any existing functionality.242-242: The
ghAccessTokenM'now usesserverGithubTokeninstead ofgithubToken. Ensure that this change does not break any existing functionality.327-371: A new function
getProfileWalletAddresshas been added. This function retrieves the wallet address associated with a given profile ID. It first checks the database for the wallet address. If it doesn't find one, it tries to get unused addresses from the wallet API. It also involves some concurrency operations usingMVar. Ensure that this function is implemented correctly and handles errors appropriately. Also, make sure that the usage ofMVarfollows best practices to avoid potential data races or deadlocks.test/ProfileWalletSpec.hs (7)
22-23: The function
createProfileWalletsis mentioned in a TODO comment but it's not defined anywhere in the provided code. Please ensure that this function is implemented or remove the comment if it's no longer relevant.28-64: This test case assumes that the address
addr_test1qqy3xyv0n4s7fm2ww7yutgzuhvvsf9mvyffmtndmmkg53er3ydg8gsfrl2eh3h26v74qvhyqvj992v7qyvqqrk2pf4kqaaahsuis in thewalletAddresseslist, which is used by theisOurAddressfunction. However, thewalletAddresseslist is not defined in the provided code. Please ensure that this list is properly initialized with the correct addresses before running the tests.66-89: Similar to the previous comment, this test case assumes that the address
addr_test1qqy3xyv0n4s7fm2ww7yutgzuhvvsf9mvyffmtndmmkg53er3ydg8gsis not in thewalletAddresseslist. Please ensure that this list is properly initialized with the correct addresses before running the tests.91-114: This test case assumes that the address
addr_test1qzj3v7pqlltaxk38c4nyv73gutm2ldl4e6atwewhwvt6yljt5w2gis in thewalletAddresseslist. Please ensure that this list is properly initialized with the correct addresses before running the tests.141-164: This test case assumes that the address
addr_test1qzj3v7pqlltaxk38c4nyv73gutm2ldl4e6atwewhwvt6yljt5w2gis in thewalletAddresseslist. Please ensure that this list is properly initialized with the correct addresses before running the tests.166-188: This test case assumes that the address
addr_test1qzj3v7pqlltaxk38c4nyv73gutm2ldl4e6atwewhwvt6yljt5w2gu33s8wrw3c9tjs97dr5pulsvf39e56v7c9ar39asaf5p6sis in thewalletAddresseslist. Please ensure that this list is properly initialized with the correct addresses before running the tests.190-204: This test case assumes that the address
addr_test1qzj3v7pqlltaxk38c4nyv73gutm2ldl4e6atwewhwvt6yljt5w2gu33s8wrw3c9tjs97dr5pulsvf39e56v7c9ar39asaf5p6sis in thewalletAddresseslist. Please ensure that this list is properly initialized with the correct addresses before running the tests.src/Plutus/Certification/Synchronizer.hs (7)
60-60: Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
85-85: Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
111-112: The change in logic from filtering out outgoing transactions to storing all transactions could potentially lead to unexpected behavior or performance issues if the number of transactions is large. Please verify that this change is intentional and tested.
148-149: The TODO comment indicates that a conversion should be removed after merging with another feature. Ensure that this has been done, or that there is a plan to do so.
166-167: The TODO comment indicates that a conversion should be removed after merging with another feature. Ensure that this has been done, or that there is a plan to do so.
276-294: The addition of
minAssignmentAmountparameter to thestartTransactionsMonitorfunction could potentially break existing calls to this function. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.308-313: The addition of
minAssignmentAmountandrefparameters to themonitorWalletTransactionsfunction could potentially break existing calls to this function. Ensure that all calls to this function throughout the codebase have been updated to match the new signature.
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| import Plutus.Certification.CertificationBroadcaster | ||
| import Observe.Event.Render.JSON | ||
| import Control.Exception | ||
| import Observe.Event.Crash | ||
|
|
||
| import qualified IOHK.Certification.Persistence as DB | ||
|
|
||
| import Data.Function (on) | ||
| import Data.UUID (UUID) | ||
| import Control.Monad.Except | ||
| import Data.Maybe (fromMaybe) | ||
| import Observe.Event.Backend | ||
| import Observe.Event | ||
| import Data.IORef | ||
| import Data.Void | ||
| import Control.Monad.RWS | ||
| import Control.Monad.Reader (ReaderT(..)) | ||
| import Observe.Event.BackendModification (setAncestor) | ||
| import Plutus.Certification.ProfileWallet | ||
| import Data.IORef | ||
| import Data.Word (Word64) | ||
| import qualified Data.HashSet as HashSet | ||
|
|
||
| import qualified Plutus.Certification.WalletClient as Wallet | ||
| import qualified IOHK.Certification.Persistence as DB |
There was a problem hiding this comment.
The import statements are not properly aligned and there is an extra space before some of the import statements. This can make the code harder to read and maintain. Please align them properly.
import Plutus.Certification.CertificationBroadcaster
import Observe.Event.Render.JSON
import Control.Exception
import Observe.Event.Crash
import Data.Function (on)
import Data.UUID (UUID)
import Control.Monad.Except
import Data.Maybe (fromMaybe)
import Observe.Event.Backend
import Observe.Event
import Data.Void
import Control.Monad.RWS
import Control.Monad.Reader (ReaderT(..))
import Observe.Event.BackendModification (setAncestor)
import Plutus.Certification.ProfileWallet
import Data.IORef
import Data.Word (Word64)
import qualified Data.HashSet as HashSet
import qualified Plutus.Certification.WalletClient as Wallet
import qualified IOHK.Certification.Persistence as DB
import Plutus.Certification.CertificationBroadcaster
import Observe.Event.Render.JSON
import Control.Exception
import Observe.Event.Crash
import Data.Function (on)
import Data.UUID (UUID)
import Control.Monad.Except
import Data.Maybe (fromMaybe)
import Observe.Event.Backend
import Observe.Event
import Data.Void
import Control.Monad.RWS
import Control.Monad.Reader (ReaderT(..))
import Observe.Event.BackendModification (setAncestor)
import Plutus.Certification.ProfileWallet
import Data.IORef
import Data.Word (Word64)
import qualified Data.HashSet as HashSet
import qualified Plutus.Certification.WalletClient as Wallet
import qualified IOHK.Certification.Persistence as DB- fix isOurAddress - add unit tests for payment
bogdan-manole
force-pushed
the
feat/dedicated-payment-addresses
branch
from
b9857cf
to
a070066
Compare
There was a problem hiding this comment.
Actionable comments generated: 2
Files selected for processing (10)
- dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1 hunks)
- plutus-certification.cabal (3 hunks)
- src/Plutus/Certification/CertificationBroadcaster.hs (4 hunks)
- src/Plutus/Certification/ProfileWallet.hs (1 hunks)
- src/Plutus/Certification/Server/Instance.hs (10 hunks)
- src/Plutus/Certification/Synchronizer.hs (11 hunks)
- src/Plutus/Certification/WalletClient.hs (5 hunks)
- test/ProfileWallet/Data.hs (1 hunks)
- test/ProfileWalletSpec.hs (1 hunks)
- test/Spec.hs (1 hunks)
Files skipped from review due to trivial changes (3)
- test/ProfileWallet/Data.hs
- test/Spec.hs
- src/Plutus/Certification/ProfileWallet.hs (diff too large)
LGTM comments (Suppressed): 5
plutus-certification.cabal (1)
- 152-165: A new test suite
plutus-certification-testhas been added. This is a good practice as it helps ensure the correctness of the code. However, make sure that the tests cover all the important scenarios and edge cases for the new features introduced in this PR.src/Plutus/Certification/Server/Instance.hs (1)
- 311-315: Empty lines have been added. This doesn't affect the functionality but improves readability.
src/Plutus/Certification/WalletClient.hs (3)
181-191: A new data type
WalletAddressInfohas been introduced along with itsFromJSONinstance. This seems to be a part of the new feature implementation and does not appear to have any issues.192-199: A new function
getWalletAddresses'has been introduced to retrieve wallet addresses. This seems to be a part of the new feature implementation and does not appear to have any issues.201-226: A new data type
WalletClienthas been introduced along with related type aliases and a functionrealClient. These changes seem to be a part of the new feature implementation and do not appear to have any issues.
Additional comments (Suppressed): 33
dapps-certification-signature-verification/src/IOHK/Certification/SignatureVerification.hs (1)
- 11-12: The addition of
bech32AddressHashandHashError(..)to the exports list is a significant change. Please ensure that these new functions are used correctly throughout the codebase, and that their error handling is properly implemented.src/Plutus/Certification/CertificationBroadcaster.hs (4)
14-14: The module name has been changed from
Plutus.Certification.TransactionBroadcastertoPlutus.Certification.CertificationBroadcaster. Please verify that all import statements across the codebase have been updated to reflect this change.33-33: The import statement for
WalletClienthas replaced the previous import ofWalletArgs. Ensure that all usages ofWalletArgsin this module have been properly replaced withWalletClient.59-63: The function signature of
createL1Certificationhas been modified. The second parameter is nowWalletClientinstead ofWalletArgs. Make sure that all calls to this function throughout the codebase have been updated to match the new signature.79-80: The function call
Wallet.broadcastTxhas replaced the previousWallet.broadcastTransaction. Confirm that the new function provides the same functionality as the old one and that it's used correctly here.plutus-certification.cabal (2)
75-75: The new module
Plutus.Certification.ProfileWallethas been added to the exposed modules. Ensure that this module is intended to be exposed and that it does not expose any sensitive or internal functionality.84-84: The module
Plutus.Certification.TransactionBroadcasterhas been renamed toPlutus.Certification.CertificationBroadcaster. Make sure all references to this module in the codebase have been updated accordingly.test/ProfileWalletSpec.hs (3)
28-41: Hardcoding values like
amountandtxin tests can make them brittle. Consider using property-based testing or defining these values in a more flexible way.66-89: This test case seems to be testing for an internal error condition rather than a specific functionality of the system. It might be better to refactor this to ensure that the system handles such errors gracefully instead of expecting them to occur.
166-188: This test case checks if the transaction amount is less than the minimum amount. However, the minimum amount is hardcoded as 10. Consider making this a configurable parameter to make the test more robust.
src/Plutus/Certification/Server/Instance.hs (9)
2-2: The addition of the
TupleSectionslanguage extension is not used anywhere in the provided code hunks. If it's not used elsewhere in the file, consider removing it to avoid unnecessary complexity.38-38: The import statement for
Plutus.Certification.Server.Internalhas been changed to use a qualified import with an aliasI. Ensure that all references to this module in the codebase have been updated accordingly.49-50: The new import statement for
Plutus.Certification.ProfileWalletasPWandData.Functorare introduced. Make sure they are used in the parts of the code not shown in the hunks.59-59: The new import statement for
Control.Concurrent (MVar, takeMVar, putMVar)is introduced. Make sure these imported functions are used in the parts of the code not shown in the hunks.80-88: The field names in the
ServerArgsdata type have been updated. Ensure that all references to these fields throughout the codebase have been updated to match the new names.118-123: The event name
I.WalletAddresshas been updated fromWalletAddress. Also, thegithubTokenvariable has been renamed toserverGithubToken. Ensure that all references to these throughout the codebase have been updated to match the new names.242-242: The
githubTokenvariable has been renamed toserverGithubToken. Ensure that all references to this variable throughout the codebase have been updated to match the new name.328-349: A new function
getProfileWalletAddresshas been added. It retrieves the wallet address associated with a profile ID. The logic seems sound, retrieving the wallet address from the database or, if not found, getting unused addresses from the wallet service. However, ensure that error handling is properly implemented in the parts of the code not shown in the hunks.352-371: Several helper functions and their logic have been updated. Ensure that these changes do not break any existing functionality and that all calls to these functions throughout the codebase have been updated accordingly.
src/Plutus/Certification/WalletClient.hs (5)
1-31: The new hunk introduces several new language extensions and exports. Ensure that these changes are compatible with the rest of the codebase and do not introduce any potential issues.
45-49: The order of imports has been changed, and
Data.Maybehas been added. This change should not cause any issues unless there are naming conflicts between the imported modules. Please verify this.99-112: The API type has been extended to include an
AddressesAPI. This is a significant change that could potentially break existing calls to the API if they have not been updated accordingly. Please ensure that all calls to the API throughout the codebase have been updated to match the new structure.150-151: The
mkClientfunction signature has been modified to include a new function for getting wallet addresses. This is a breaking change that could potentially affect existing calls tomkClient. Please ensure that all calls tomkClientthroughout the codebase have been updated to match the new signature.160-171: The
broadcastTransaction'function now includes an optional destination address parameter. This is a breaking change that could potentially affect existing calls tobroadcastTransaction'. Please ensure that all calls tobroadcastTransaction'throughout the codebase have been updated to match the new signature.src/Plutus/Certification/Synchronizer.hs (9)
29-49: The import statements have been rearranged and new ones have been added. Ensure that all the imported modules are used in the code and there are no unused imports. Also, verify that the refactoring of
Plutus.Certification.TransactionBroadcastertoPlutus.Certification.CertificationBroadcasterhas been done correctly throughout the codebase.60-60: A new selector
InjectProfileWalletSynchas been introduced. Make sure it is used appropriately in the rest of the code.85-85: The function
renderSynchronizerSelectornow handles the case forInjectProfileWalletSync. Ensure thatrenderProfileWalletSyncSelectoris defined and works as expected.111-112: The function
synchronizeDbTransactionshas been modified to store all transactions instead of just incoming ones. This change could potentially affect other parts of the system that rely on this function. Verify that this change does not introduce any unintended side effects.148-149: The conversion from
txOutputAmount.quantitytofromIntegral txOutputAmount.quantitymight cause precision loss ifquantityis a floating-point number. Please confirm if this conversion is necessary and won't lead to precision loss.166-167: Similar to the previous comment, the conversion from
txOutputAmount.quantitytofromIntegral txOutputAmount.quantitymight cause precision loss ifquantityis a floating-point number. Please confirm if this conversion is necessary and won't lead to precision loss.276-294: The function
startTransactionsMonitorandstartTransactionsMonitor'have been updated with a new parameterminAssignmentAmount. Ensure that all calls to these functions throughout the codebase have been updated to match the new signature.258-258: The conversion from
run.certificationPricetofromIntegral run.certificationPricemight cause precision loss ifcertificationPriceis a floating-point number. Please confirm if this conversion is necessary and won't lead to precision loss.308-313: The function
doWorkhas been updated to includerefwhich is an IORef to a list. It's not clear what this reference is used for. Please provide more context or comments explaining its purpose.
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| isOurAddress :: Text -> Bool | ||
| isOurAddress = flip Prelude.elem walletAddresses | ||
|
|
||
| -- TODO: test createProfileWallets |
There was a problem hiding this comment.
The function isOurAddress is defined but not used in the code. If it's not needed, consider removing it to avoid confusion.
- isOurAddress :: Text -> Bool
- isOurAddress = flip Prelude.elem walletAddresses| -- IMPORTANT: based on the direction of the transaction | ||
| -- we set the amount to be positive or negative | ||
| , DB.wtxAmount = amountDirection * fromIntegral ( walletTxData.walletTxAmount.quantity ) | ||
| , DB.wtxTime = time | ||
| , DB.wtxDepth = maybe (-1) quantity (walletTxData.walletTxDepth) | ||
| , DB.wtxStatus = walletTxStatusToDbStatus walletTxStatus | ||
| , DB.wtxMetadata = case walletTxData.walletTxMetadata of | ||
| Nothing -> "" | ||
| Just val -> decodeUtf8 . toStrict . encode $ val | ||
| } | ||
| amountDirection = if walletTxData.walletTxDirection == Incoming then 1 else (-1) |
There was a problem hiding this comment.
The calculation of DB.wtxAmount has been changed to consider the direction of the transaction. This is a significant change and should be thoroughly tested to ensure it behaves as expected.
bogdan-manole
force-pushed
the
feat/dedicated-payment-addresses
branch
from
72ab186
to
85093da
Compare
There was a problem hiding this comment.
Actionable comments generated: 0
Files selected for processing (4)
- nix/ci.nix (1 hunks)
- nix/docker-files/docker.nix (3 hunks)
- nix/docker-files/run.sh (1 hunks)
- server/Main.hs (8 hunks)
Files skipped from review due to trivial changes (2)
- nix/ci.nix
- nix/docker-files/run.sh
LGTM comments (Suppressed): 1
server/Main.hs (1)
- 165-176: The parser for command-line arguments has been updated to include a new option
min-amount-for-address-reservation. This change seems fine as it provides a way to set the minimum amount for address reservation via command-line arguments.
Additional comments (Suppressed): 8
nix/docker-files/docker.nix (3)
4-4: The tag for the Docker image has been updated from "12" to "13". Ensure that all references to this Docker image in your deployment scripts, CI/CD pipelines, and documentation have been updated accordingly.
34-34: A new environment variable
MIN_AMOUNT_FOR_ADDRESS_RESERVATIONis being added. Make sure this environment variable is properly set in the environments where this Docker image will be deployed. Also, ensure that the application handles cases where this environment variable might not be set.137-137: The environment variable
MIN_AMOUNT_FOR_ADDRESS_RESERVATIONis being used here. As mentioned earlier, make sure it's properly set in the environments where this Docker image will be deployed.server/Main.hs (5)
60-80: The import of
Plutus.Certification.ProfileWalletandData.Wordare new additions to the code. Ensure that these modules exist and are accessible from this file's location. Also, verify that the functions or types imported from these modules are used in the code.90-94: A new field
minAmountForAddressAssignmenthas been added to theArgsdata type. Make sure all instances whereArgsis used have been updated to include this new field.443-449: A new variable
addressRotationhas been introduced and initialized with an emptyAddressRotation. Ensure that this variable is used appropriately in the rest of the codebase.452-458: The server context now includes
addressRotation. Ensure that all calls toserverhave been updated to pass this new argument.495-512: The function
startTransactionsMonitornow takes an additional argumentargs.minAmountForAddressAssignment. Ensure that all calls to this function have been updated to pass this new argument.
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
There was a problem hiding this comment.
Actionable comments generated: 0
Files selected for processing (4)
- nix/ci.nix (1 hunks)
- nix/docker-files/docker.nix (3 hunks)
- nix/docker-files/run.sh (1 hunks)
- server/Main.hs (8 hunks)
Files skipped from review due to trivial changes (2)
- nix/ci.nix
- nix/docker-files/run.sh
Additional comments (Suppressed): 9
nix/docker-files/docker.nix (3)
1-7: The tag of the Docker image has been updated from "12" to "13". Ensure that all references to this Docker image in your deployment scripts, CI/CD pipelines, and documentation have been updated to use the new tag.
31-34: A new environment variable
MIN_AMOUNT_FOR_ADDRESS_RESERVATIONis being added as a parameter. Make sure this environment variable is properly set in the environments where this Docker image will be run.134-137: The environment variable
MIN_AMOUNT_FOR_ADDRESS_RESERVATIONis being used. Ensure that it is correctly set in the Docker environment.server/Main.hs (6)
60-80: The import of
Plutus.Certification.ProfileWalletandData.Wordare new additions to the code. Ensure that these modules exist and are accessible from this file's location. Also, ensure that the functions or data types used from these modules are correctly implemented.90-94: A new field
minAmountForAddressAssignmenthas been added to theArgsdata type. Make sure all instances whereArgsis used have been updated to include this new field.165-176: A new command-line argument
min-amount-for-address-reservationhas been introduced. This seems to be related to the new feature for dedicated payment addresses. Ensure that this argument is properly handled in the application logic.443-447: A new variable
addressRotationhas been introduced and initialized withemptyAddressRotation. Ensure that this variable is used appropriately in the subsequent code and its state is managed correctly throughout the application lifecycle.495-500: A new function
startTransactionsMonitoris being called with a new parameterargs.minAmountForAddressAssignment. Ensure that this function handles the new parameter correctly and doesn't introduce any side effects.501-512: In the
serverArgsfunction, a new parameterserverAddressRotationhas been added. Make sure all calls to this function have been updated to include this new parameter.
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
There was a problem hiding this comment.
Actionable comments generated: 1
Files selected for processing (1)
- src/Plutus/Certification/Synchronizer.hs (9 hunks)
Additional comments (Suppressed): 6
src/Plutus/Certification/Synchronizer.hs (6)
54-63: The
ErrorFielddata constructor inInitializingFieldhas been changed fromIOExceptiontoSomeException. This change broadens the type of exceptions that can be handled, which might be beneficial if there are other types of exceptions that need to be caught. However, it could also potentially hide bugs by catching and handling exceptions that should not occur. Please verify that this change is intentional and appropriate.108-115: The logic for filtering out outgoing transactions before syncing them with the database has been removed. If this was intentional, please ensure that the new behavior is correct and doesn't introduce any issues.
119-134: The calculation for
DB.wtxAmounthas been updated to consider the direction of the transaction. This is a significant change and should be verified to ensure it doesn't introduce any issues or inaccuracies in the transaction amounts.163-170: The conversion from
txOutputAmount.quantitytofromIntegral txOutputAmount.quantityhas been added. This change seems to be related to a TODO comment about removing the conversion after merging withfeat/subscription. Please ensure that this conversion is necessary and correctly implemented.255-261: The cost of the run is now calculated using
fromIntegral run.certificationPriceinstead ofrun.certificationPrice. This change seems to be related to a change in the type ofcertificationPrice. Please ensure that this conversion is necessary and correctly implemented.273-294: The
startTransactionsMonitorfunction now takes an additional argumentminAssignmentAmount. Please ensure that all calls to this function throughout the codebase have been updated to match the new signature.
Tips
Chat with CodeRabbit Bot (@coderabbitai)
- Tag the bot with
@coderabbitaiin any review comment to ask it questions. The bot will not respond unless explicitly tagged. - A review comment is a comment on a diff or a file and not a comment on the PR itself.
- You can use the mouse to select multiple lines of code to provide exact context to the bot.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| import Control.Monad.Catch (MonadMask, catchAll) | ||
| import Data.List (groupBy) | ||
| import Plutus.Certification.API.Routes (RunIDV1(..)) | ||
| import Plutus.Certification.CoinGeckoClient | ||
| import Data.Aeson | ||
| import Plutus.Certification.TransactionBroadcaster | ||
| import Plutus.Certification.CertificationBroadcaster | ||
| import Observe.Event.Render.JSON | ||
| import Control.Exception | ||
| import Observe.Event.Crash | ||
|
|
||
| import qualified IOHK.Certification.Persistence as DB | ||
|
|
||
| import Data.Function (on) | ||
| import Data.UUID (UUID) | ||
| import Control.Monad.Except | ||
| import Data.Maybe (fromMaybe) | ||
| import Observe.Event.Backend | ||
| import Observe.Event | ||
| import Data.IORef | ||
| import Data.Void | ||
| import Control.Monad.RWS | ||
| import Control.Monad.Reader (ReaderT(..)) | ||
| import Observe.Event.BackendModification (setAncestor) | ||
| import Plutus.Certification.ProfileWallet |
There was a problem hiding this comment.
The import statements are not sorted and some of them are indented inconsistently. It's a good practice to sort the imports alphabetically and maintain consistent indentation for better readability.
- import Data.Time (UTCTime)
- import Data.ByteString (toStrict)
- import Data.Text.Encoding (decodeUtf8)
- import Control.Monad.Catch (MonadMask, catchAll)
- import Data.List (groupBy)
- import Plutus.Certification.API.Routes (RunIDV1(..))
- import Plutus.Certification.CoinGeckoClient
- import Data.Aeson
- import Plutus.Certification.CertificationBroadcaster
- import Observe.Event.Render.JSON
- import Control.Exception
- import Observe.Event.Crash
- import Data.Function (on)
- import Data.UUID (UUID)
- import Control.Monad.Except
- import Data.Maybe (fromMaybe)
- import Observe.Event.Backend
- import Observe.Event
- import Data.Void
- import Control.Monad.RWS
- import Control.Monad.Reader (ReaderT(..))
- import Observe.Event.BackendModification (setAncestor)
- import Plutus.Certification.ProfileWallet
- import Data.IORef
- import Data.Word (Word64)
- import qualified Data.HashSet as HashSet
+import Control.Exception
+import Control.Monad.Catch (MonadMask, catchAll)
+import Control.Monad.Except
+import Control.Monad.RWS
+import Control.Monad.Reader (ReaderT(..))
+import Data.Aeson
+import Data.ByteString (toStrict)
+import Data.Function (on)
+import Data.IORef
+import Data.List (groupBy)
+import Data.Maybe (fromMaybe)
+import Data.Text.Encoding (decodeUtf8)
+import Data.Time (UTCTime)
+import Data.UUID (UUID)
+import Data.Void
+import Data.Word (Word64)
+import Observe.Event
+import Observe.Event.Backend
+import Observe.Event.BackendModification (setAncestor)
+import Observe.Event.Crash
+import Observe.Event.Render.JSON
+import Plutus.Certification.API.Routes (RunIDV1(..))
+import Plutus.Certification.CertificationBroadcaster
+import Plutus.Certification.CoinGeckoClient
+import Plutus.Certification.ProfileWallet
+import qualified Data.HashSet as HashSet
Summary by CodeRabbit
ProfileWalletmodule for managing profile wallets, including address assignment and wallet synchronization.bech32AddressHashfunction andHashErrordata type for hashing operations.Plutus.Certification.TransactionBroadcastertoPlutus.Certification.CertificationBroadcasterand updated related functions.ServerArgsdata type inPlutus.Certification.Server.Instancewith new fields and renamed existing ones for clarity.WalletClientinPlutus.Certification.WalletClientwith new endpoints and data types.ProfileWalletSpectest suite for testing theProfileWalletmodule.