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

Review, finalize, and harden core trade protocol and code #28

Open
woodser opened this issue Apr 17, 2021 · 2 comments
Open

Review, finalize, and harden core trade protocol and code #28

woodser opened this issue Apr 17, 2021 · 2 comments
Labels

Comments

@woodser
Copy link
Contributor

woodser commented Apr 17, 2021

There are many details to consider, finalize, and harden in the core trade protocol and code.

Priorities

  • stress test
  • implement scheduling for trades, other
  • trade completion refactoring
  • auto start seed node
  • auto start funding wallet
  • never wait for onArrived() or ack in protocols? (ProcessDepositResponse, ArbittratorSendsInitTrade*, ProcessInitMultisigRequest)
  • allow node address to change; temp node address vs trusting/persisting into trade model
  • test recovery from disconnecting or sleeping at various stages (balances stop updating)
  • understand how arbitrator key is revoked, ensure new trader can verify past signatures and know if revoked (see bannedDisputeAgents)
  • arbitrator api to register keys and resolve dispute?
  • all api calls denominated in xmr atomic units
  • show percent progress syncing daemon and wallet on startup
  • wallet rpc authentication based on user/wallet password?
  • test offers with trade range
  • allow offers with multiple payment methods?
  • do not update offers which would invalidate arbitrator signature (OpenOfferManager.maybeUpdatePersistedOffers())
  • arbitrator TradeStepView errors / payment started button during trade
  • connection manager autodetect peer connections
  • signatures can be bytes per DipsuteResult.abitrator_signature
  • btc atomic swaps
  • funding wallet needs manually funded if cannot mine to it
  • in offer counterCurrencyCode='XMR', baseCurrencyCode='ETH' ?
  • CurrencyUtil:435 base currency and counter currency switched
  • loss of precision handling BigInteger as long? e.g. XmrBalanceInfo? switch to strings in pb?
  • test creating hundreds of new deposit addresses
  • switch to native wallets?
  • arbitrator broadcasts reserve tx if offer opened with same tx key within e.g. 1 second
  • changeWallet() opens and syncs every wallet at once
  • limit maximum number of wallets open at a time
  • increase sync period (e.g. 10 or 15 seconds) for remote daemon connections
  • backup multisig wallets of open trades
  • "We found an outdated addressEntry for openOffer" for past trades
  • delete multisig wallets if payout not confirmed after X blocks or when payout tx unlocks
  • test that funds unreserve if arbitrator doesn't sign offer
  • bitcoinj warning about threading
  • full code refactor from big picture architecture
  • use proof of reserves instead of specific key images?
  • performance analysis of initializing trade
  • TradeManager.closeDisputedTrade() should be called by arbitrator when dispute closed
  • ArbitratorSendsInitTradeAndMultisigRequests sends multisig requests on ack but should skip if maker not reserved
  • test completing trade with peers offline between messages
  • switch ui tests to logging library
  • send PayoutTxPublishedMessage to peer and arbitrator once signed
  • test resolving dispute after payment started
  • refactor dispute resolution; dispute opener includes updated multisig hex, arbitrator sends payout tx and updated multisig hex to opener, opener signs, broadcasts payout, and sends response / PayoutTxPublishedMessage
  • optimize keeping wallets open and syncing, or syncing on unlock and then not re-syncing
  • ensure OPenDispute ack comes back before re-opening/syncing wallet; analyze when all all wallets opened, closed, synced

Trade Completion

  • review payout process
  • Certify and tabulate completed trades #143
  • support arbitration fee and reimbursing trader fee
  • verify payout txs
  • accurate fee estimation for tx verification
  • close arbitrator trade at completion
  • test arbitrator trade state in api tests
  • don't delete trade wallets until payout tx unlocks

Optimizations / Performance

  • optimize offer key image verification (only verify offers presented to ui? batch key image verification?)
  • optimize number of open wallets and refresh period, especially arbitrator (see TradeManager.initPersistedTrades(), XmrWalletService.getMultisigWallet())

API Functions

Needs Consideration

  • completely replace errorMessageHandler with regular exception handling
  • send payment info on deposit unlock instead of deposit broadcast?
  • clarify arbitrator/mediator/refund agent terminology and use throughout application
  • switch base unit long from centineros to atomic units
  • "No refund agents available" after idling
  • "We found a closed trade with locked up funds" error
  • "Wallet missing deposit tx" error
  • "We had an old offer in the list with the same offer id"
  • one TraderProtocol which supports maker, taker, buyer, seller methods to prevent duplicate implementations?
  • flatten ProcessModel into Trade / other model refactors?
  • refactor State/Phase for CleanupTradeableOnFault
  • test retaking failed trade. change naming convention and create new multisig wallet or replace? cannot reuse
  • TradeDataValidation.validateDepositInputs() needs updated for XMR, consolidate tx validation there?
  • Create multisigs as offsets from main mnemonic, allow users to recreate multisig?

Note additional core todos are throughout the code prefixed with TODO (woodser).

@erciccione erciccione added Epic needs:discussion this issue needs a discussion labels Apr 17, 2021
@erciccione
Copy link
Contributor

pinging @haveno-dex/developers for inputs.

@erciccione erciccione added the P1 top priority. Should be fixed ASAP label May 4, 2021
@erciccione erciccione added a:network Changes related to the p2p network and Tor a:trade process Issue about the trade process is:feature Request for a new feature is:refactor issue about refactoring and removed P1 top priority. Should be fixed ASAP needs:discussion this issue needs a discussion labels May 16, 2021
@erciccione
Copy link
Contributor

removing needs:discussion label. As this is an internal todo listi and some of its content will be split into single issues later on.

@erciccione erciccione removed is:feature Request for a new feature a:network Changes related to the p2p network and Tor a:trade process Issue about the trade process is:refactor issue about refactoring labels Sep 8, 2021
@erciccione erciccione pinned this issue Sep 8, 2021
@erciccione erciccione unpinned this issue Oct 27, 2021
@erciccione erciccione pinned this issue Nov 9, 2021
@erciccione erciccione unpinned this issue Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants