diff --git a/cmd/swapd/main.go b/cmd/swapd/main.go index ab961234..263be45e 100644 --- a/cmd/swapd/main.go +++ b/cmd/swapd/main.go @@ -187,7 +187,8 @@ var ( }, &cli.BoolFlag{ Name: flagTransferBack, - Usage: "When receiving XMR in a swap, transfer it back to the original wallet.", + Usage: "Set to false to leave XMR in generated swap wallet instead of moving to primary.", + Value: true, }, &cli.StringFlag{ Name: flagLogLevel, @@ -675,11 +676,9 @@ func getProtocolInstances(c *cli.Context, cfg common.Config, walletPassword := c.String(flagMoneroWalletPassword) xmrtakerCfg := &xmrtaker.Config{ - Backend: b, - DataDir: cfg.DataDir, - MoneroWalletFile: walletFilePath, - MoneroWalletPassword: walletPassword, - TransferBack: c.Bool(flagTransferBack), + Backend: b, + DataDir: cfg.DataDir, + TransferBack: c.Bool(flagTransferBack), } xmrtaker, err := xmrtaker.NewInstance(xmrtakerCfg) diff --git a/cmd/swaptester/main.go b/cmd/swaptester/main.go index f1bf8bbe..4288870e 100644 --- a/cmd/swaptester/main.go +++ b/cmd/swaptester/main.go @@ -31,8 +31,8 @@ const ( flagLogLevel = "log-level" flagDev = "dev" - defaultConfigFile = "testerconfig.json" - defaultXMRMakerMoneroEndpoint = "http://127.0.0.1:18083/json_rpc" + defaultConfigFile = "testerconfig.json" + defaultXMRMakerMoneroWalletPort = 18083 ) var ( @@ -120,7 +120,11 @@ func runTester(c *cli.Context) error { if !isDev { // TODO: Why do this when dev flag is not given? Can the code work if it is given? // For this to work, you'll need to pass --wallet-port 18083 to the XMR Maker swapd - defaultMoneroClient = monero.NewThinWalletClient(defaultXMRMakerMoneroEndpoint) + defaultMoneroClient = monero.NewThinWalletClient( + "127.0.0.1", + common.DefaultMoneroDaemonDevPort, + defaultXMRMakerMoneroWalletPort, + ) } var timeout time.Duration @@ -196,7 +200,7 @@ func getRandomExchangeRate() types.ExchangeRate { } func generateBlocks() { - cXMRMaker := monero.NewThinWalletClient(defaultXMRMakerMoneroEndpoint) + cXMRMaker := defaultMoneroClient xmrmakerAddr, err := cXMRMaker.GetAddress(0) if err != nil { log.Errorf("failed to get default monero address: %s", err) @@ -398,7 +402,7 @@ func (d *daemon) makeOffer(done <-chan struct{}) { if isDev { generateBlocks() } else { - _, err := monero.WaitForBlocks(defaultMoneroClient, 10) + _, err := monero.WaitForBlocks(context.Background(), defaultMoneroClient, 10) if err != nil { log.Errorf("failed to wait for blocks: %s", err) } diff --git a/common/config.go b/common/config.go index 70fca81f..372516ae 100644 --- a/common/config.go +++ b/common/config.go @@ -42,7 +42,7 @@ var StagenetConfig = Config{ DataDir: path.Join(baseDir, "stagenet"), MoneroDaemonHost: "node.sethforprivacy.com", MoneroDaemonPort: 38089, // Seth is not using the default stagenet value of 38081 (so don't use our constant) - ContractAddress: ethcommon.HexToAddress("0x64e902cD8A29bBAefb9D4e2e3A24d8250C606ee7"), + ContractAddress: ethcommon.HexToAddress("0x5F8Cf66C4c59d398052aA75D46Ce48e7fA09A83E"), Bootnodes: []string{ "/ip4/134.122.115.208/tcp/9900/p2p/12D3KooWDqCzbjexHEa8Rut7bzxHFpRMZyDRW1L6TGkL1KY24JH5", "/ip4/143.198.123.27/tcp/9900/p2p/12D3KooWSc4yFkPWBFmPToTMbhChH3FAgGH96DNzSg5fio1pQYoN", diff --git a/common/utils.go b/common/utils.go index 7c33336e..0f8b49c9 100644 --- a/common/utils.go +++ b/common/utils.go @@ -1,9 +1,11 @@ package common import ( + "context" "crypto/ecdsa" "fmt" "os" + "time" ethcommon "github.com/ethereum/go-ethereum/common" ethcrypto "github.com/ethereum/go-ethereum/crypto" @@ -56,3 +58,18 @@ func FileExists(path string) (bool, error) { return false, err } + +// SleepWithContext is the same as time.Sleep(...) but with preemption if the context is +// complete. Returns nil if the sleep completed, otherwise the context's error. +func SleepWithContext(ctx context.Context, d time.Duration) error { + timer := time.NewTimer(d) + select { + case <-ctx.Done(): + if !timer.Stop() { + <-timer.C + } + return ctx.Err() + case <-timer.C: + return nil + } +} diff --git a/common/utils_test.go b/common/utils_test.go index 2dd519b0..47b49590 100644 --- a/common/utils_test.go +++ b/common/utils_test.go @@ -1,9 +1,13 @@ package common import ( + "context" + "io/fs" + "io/ioutil" "os" "path" "testing" + "time" ethcommon "github.com/ethereum/go-ethereum/common" "github.com/stretchr/testify/assert" @@ -35,3 +39,48 @@ func TestMakeDir(t *testing.T) { require.NoError(t, err) assert.Equal(t, "drwx------", fileStats.Mode().String()) // only user has access } + +func TestFileExists(t *testing.T) { + tmpDir := t.TempDir() + presentFile := path.Join(tmpDir, "file-is-here.txt") + missingFile := path.Join(tmpDir, "no-file-here.txt") + noAccessFile := path.Join(tmpDir, "no-access", "any-file.txt") + + // file exists + require.NoError(t, ioutil.WriteFile(presentFile, nil, 0600)) + exists, err := FileExists(presentFile) + require.NoError(t, err) + assert.True(t, exists) + + // file does not exist + exists, err = FileExists(missingFile) + require.NoError(t, err) + assert.False(t, exists) + + // no access to know if the file exists + require.NoError(t, os.Mkdir(path.Dir(noAccessFile), 0000)) // no access permissions on dir + _, err = FileExists(noAccessFile) + require.ErrorIs(t, err, fs.ErrPermission) + + // path present, but it is a directory instead of a file + _, err = FileExists(tmpDir) + require.Error(t, err) + require.Contains(t, err.Error(), "directory") +} + +// Checks normal, non-cancelled operation +func TestSleepWithContext_fullSleep(t *testing.T) { + ctx := context.Background() + err := SleepWithContext(ctx, -1*time.Hour) // negative duration doesn't sleep or panic + assert.NoError(t, err) + err = SleepWithContext(ctx, 10*time.Millisecond) + assert.NoError(t, err) +} + +// Checks that we handle context cancellation and break out of the sleep +func TestSleepWithContext_canceled(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) + defer cancel() + err := SleepWithContext(ctx, 24*time.Hour) // time out the test if we fail + assert.ErrorIs(t, err, context.DeadlineExceeded) +} diff --git a/ethereum/block/wait_for_receipt.go b/ethereum/block/wait_for_receipt.go index 1519dabe..34bbbfe4 100644 --- a/ethereum/block/wait_for_receipt.go +++ b/ethereum/block/wait_for_receipt.go @@ -10,6 +10,8 @@ import ( ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/ethclient" logging "github.com/ipfs/go-log" + + "github.com/athanorlabs/atomic-swap/common" ) const ( @@ -30,7 +32,9 @@ func WaitForReceipt(ctx context.Context, ec *ethclient.Client, txHash ethcommon. receipt, err := ec.TransactionReceipt(ctx, txHash) if err != nil { log.Infof("waiting for transaction to be included in chain: txHash=%s", txHash) - time.Sleep(receiptSleepDuration) + if err = common.SleepWithContext(ctx, receiptSleepDuration); err != nil { + return nil, err + } continue } if receipt.Status != ethtypes.ReceiptStatusSuccessful { diff --git a/ethereum/block/wait_for_timestamp.go b/ethereum/block/wait_for_timestamp.go index bc00eb16..d7cca291 100644 --- a/ethereum/block/wait_for_timestamp.go +++ b/ethereum/block/wait_for_timestamp.go @@ -6,19 +6,9 @@ import ( ethtypes "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/ethclient" -) -// sleepWithContext is the same as time.Sleep(...) but with preemption if the context is complete. -func sleepWithContext(ctx context.Context, d time.Duration) { - timer := time.NewTimer(d) - select { - case <-ctx.Done(): - if !timer.Stop() { - <-timer.C - } - case <-timer.C: - } -} + "github.com/athanorlabs/atomic-swap/common" +) // WaitForEthBlockAfterTimestamp returns the header of the first block whose timestamp is >= ts. func WaitForEthBlockAfterTimestamp(ctx context.Context, ec *ethclient.Client, ts int64) (*ethtypes.Header, error) { @@ -26,7 +16,9 @@ func WaitForEthBlockAfterTimestamp(ctx context.Context, ec *ethclient.Client, ts // The sleep is safe even if timeDelta is negative. We only optimise for timestamps in the future, but if // the timestamp had already passed for some reason, nothing bad happens. - sleepWithContext(ctx, timeDelta) + if err := common.SleepWithContext(ctx, timeDelta); err != nil { + return nil, err + } // subscribe to new block headers headers := make(chan *ethtypes.Header) diff --git a/ethereum/block/wait_for_timestamp_test.go b/ethereum/block/wait_for_timestamp_test.go index ed0e4636..f48589b9 100644 --- a/ethereum/block/wait_for_timestamp_test.go +++ b/ethereum/block/wait_for_timestamp_test.go @@ -10,20 +10,6 @@ import ( "github.com/athanorlabs/atomic-swap/tests" ) -// Checks normal, non-cancelled operation -func TestSleepWithContext_fullSleep(t *testing.T) { - ctx := context.Background() - sleepWithContext(ctx, -1*time.Hour) // negative duration doesn't sleep or panic - sleepWithContext(ctx, 10*time.Millisecond) -} - -// Checks that we handle context cancellation and break out of the sleep -func TestSleepWithContext_canceled(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Millisecond) - defer cancel() - sleepWithContext(ctx, 24*time.Hour) // time out the test if we fail -} - // Tests the normal, full flow where we subscribe to new headers and quit after finding // a header with stamp >= ts. func TestWaitForEthBlockAfterTimestamp_smallWait(t *testing.T) { diff --git a/go.mod b/go.mod index bcbb1d5d..3a08b8dc 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/AthanorLabs/go-relayer v0.0.0-20221022234503-174bb7b32389 github.com/AthanorLabs/go-relayer-client v0.0.0-20220929181539-02b89bc5e882 github.com/ChainSafe/chaindb v0.1.5-0.20221010190531-f900218c88f8 - github.com/MarinX/monerorpc v1.0.2 + github.com/MarinX/monerorpc v1.0.4 github.com/athanorlabs/cgo-dleq v0.0.0-20220929204103-ca62cc9baa28 github.com/btcsuite/btcd/btcutil v1.1.2 github.com/chyeh/pubip v0.0.0-20170203095919-b7e679cf541c diff --git a/go.sum b/go.sum index be7c6930..0cd6ef1f 100644 --- a/go.sum +++ b/go.sum @@ -55,8 +55,10 @@ github.com/ChainSafe/log15 v1.0.0 h1:vRDVtWtVwIH5uSCBvgTTZh6FA58UBJ6+QiiypaZfBf8 github.com/ChainSafe/log15 v1.0.0/go.mod h1:5v1+ALHtdW0NfAeeoYyKmzCAMcAeqkdhIg4uxXWIgOg= github.com/DataDog/zstd v1.4.1 h1:3oxKN3wbHibqx897utPC2LTQU4J+IHWWJO+glkAkpFM= github.com/DataDog/zstd v1.4.1/go.mod h1:1jcaCB/ufaK+sKp1NBhlGmpz41jOoPQ35bpF36t7BBo= -github.com/MarinX/monerorpc v1.0.2 h1:q8t05wco0L/gvYPx0j+O/RaUBhrLVC41yQWklHXgpGg= -github.com/MarinX/monerorpc v1.0.2/go.mod h1:NohAIf5kJ4pS0sO9mbEQkI1dLHuxd4L0DX2Zou0Yofo= +github.com/MarinX/monerorpc v1.0.3 h1:ZVxOyZn609yp+Xn1I6G38WgsvCxSrWqJ5JE1qtwq5Rw= +github.com/MarinX/monerorpc v1.0.3/go.mod h1:NohAIf5kJ4pS0sO9mbEQkI1dLHuxd4L0DX2Zou0Yofo= +github.com/MarinX/monerorpc v1.0.4 h1:p1ui0bD8s7tdC1zbFy3UacwDCyBo5wCMa1aVfDPAgxI= +github.com/MarinX/monerorpc v1.0.4/go.mod h1:NohAIf5kJ4pS0sO9mbEQkI1dLHuxd4L0DX2Zou0Yofo= github.com/OneOfOne/xxhash v1.2.2 h1:KMrpdQIwFcEqXDklaen+P1axHaj9BSKzvpUUfnHldSE= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/VictoriaMetrics/fastcache v1.6.0 h1:C/3Oi3EiBCqufydp1neRZkqcwmEiuRT9c3fqvvgKm5o= diff --git a/monero/mine_regtest.go b/monero/mine_regtest.go index dd546c13..d08822af 100644 --- a/monero/mine_regtest.go +++ b/monero/mine_regtest.go @@ -13,7 +13,7 @@ const ( // MonerodRegtestEndpoint is the RPC endpoint used by monerod in the dev environment's regtest mode. MonerodRegtestEndpoint = "http://127.0.0.1:18081/json_rpc" - backgroundMineInterval = 3 * time.Second + backgroundMineInterval = 1 * time.Second errBlockNotAccepted = "Block not accepted" ) @@ -26,9 +26,10 @@ var mineMu sync.Mutex func BackgroundMineBlocks(ctx context.Context, blockRewardAddress string) { var wg sync.WaitGroup wg.Add(1) - defer wg.Wait() + // Lower the sleep duration used by WaitForBlock + blockSleepDuration = backgroundMineInterval / 3 go func() { defer wg.Done() if !mineMu.TryLock() { @@ -37,7 +38,6 @@ func BackgroundMineBlocks(ctx context.Context, blockRewardAddress string) { defer mineMu.Unlock() for { - time.Sleep(backgroundMineInterval) select { case <-ctx.Done(): return @@ -46,7 +46,7 @@ func BackgroundMineBlocks(ctx context.Context, blockRewardAddress string) { } daemonCli := monerorpc.New(MonerodRegtestEndpoint, nil).Daemon - _, err := daemonCli.GenerateBlocks(&daemon.GenerateBlocksRequest{ + resp, err := daemonCli.GenerateBlocks(&daemon.GenerateBlocksRequest{ AmountOfBlocks: 1, WalletAddress: blockRewardAddress, }) @@ -55,10 +55,11 @@ func BackgroundMineBlocks(ctx context.Context, blockRewardAddress string) { // blocks, not an error that matters unless it is happening frequently. continue } else if err != nil { - log.Warnf("failed to mine block: %s", err) + log.Warnf("Failed to mine block: %s", err) + } + if false { // change to true if debugging and you want to see when new blocks are generated + log.Debugf("Background mined 1 monero block at height=%d", resp.Height) } - - log.Debugf("background mined 1 monero block") } }() } diff --git a/monero/test_support.go b/monero/test_support.go index 1ea40a48..c72d1238 100644 --- a/monero/test_support.go +++ b/monero/test_support.go @@ -24,6 +24,11 @@ import ( "github.com/athanorlabs/atomic-swap/common" ) +const ( + // Mastering monero example address (we don't use the background mining block rewards in tests) + blockRewardAddress = "4BKjy1uVRTPiz4pHyaXXawb82XpzLiowSDd8rEQJGqvN6AD6kWosLQ6VJXW9sghopxXgQSh1RTd54JdvvCRsXiF41xvfeW5" +) + // CreateWalletClientWithWalletDir creates a WalletClient with the given wallet directory. func CreateWalletClientWithWalletDir(t *testing.T, walletDir string) WalletClient { _, filename, _, ok := runtime.Caller(0) // this test file path @@ -67,8 +72,6 @@ func GetBalance(t *testing.T, wc WalletClient) *wallet.GetBalanceResponse { // that is in regtest mode. If there is an existing go routine that is already mining from // a previous call, no new go routine is created. func TestBackgroundMineBlocks(t *testing.T) { - const blockRewardAddress = "4BKjy1uVRTPiz4pHyaXXawb82XpzLiowSDd8rEQJGqvN6AD6kWosLQ6VJXW9sghopxXgQSh1RTd54JdvvCRsXiF41xvfeW5" //nolint:lll - var wg sync.WaitGroup wg.Add(1) ctx, cancelFunc := context.WithCancel(context.Background()) @@ -76,6 +79,8 @@ func TestBackgroundMineBlocks(t *testing.T) { cancelFunc() wg.Wait() }) + // Lower the sleep duration used by WaitForBlock + blockSleepDuration = backgroundMineInterval / 3 go func() { defer wg.Done() if !mineMu.TryLock() { @@ -83,7 +88,6 @@ func TestBackgroundMineBlocks(t *testing.T) { } defer mineMu.Unlock() for { - time.Sleep(backgroundMineInterval) select { case <-ctx.Done(): return diff --git a/monero/utils.go b/monero/utils.go index 8cdc8cc0..0603c10a 100644 --- a/monero/utils.go +++ b/monero/utils.go @@ -1,6 +1,7 @@ package monero import ( + "context" "fmt" "time" @@ -10,44 +11,47 @@ import ( logging "github.com/ipfs/go-log" ) -const ( - maxRetries = 360 - blockSleepDuration = time.Second * 10 -) - var ( + // blockSleepDuration is the duration that we sleep between checks for new blocks. We + // lower it in dev environments if fast background mining is started. + blockSleepDuration = time.Second * 10 + log = logging.Logger("monero") ) // WaitForBlocks waits for `count` new blocks to arrive. // It returns the height of the chain. -func WaitForBlocks(client WalletClient, count int) (uint64, error) { - prevHeight, err := client.GetHeight() +func WaitForBlocks(ctx context.Context, client WalletClient, count int) (uint64, error) { + startHeight, err := client.GetChainHeight() if err != nil { return 0, fmt.Errorf("failed to get height: %w", err) } + prevHeight := startHeight - 1 // prevHeight is only for logging + endHeight := startHeight + uint64(count) - for j := 0; j < count; j++ { - for i := 0; i < maxRetries; i++ { - if err := client.Refresh(); err != nil { - return 0, err - } + for { + if err := client.Refresh(); err != nil { + return 0, err + } - height, err := client.GetHeight() - if err != nil { - continue - } + height, err := client.GetChainHeight() + if err != nil { + return 0, err + } - if height > prevHeight { - return height, nil - } + if height >= endHeight { + return height, nil + } - log.Infof("waiting for next block, current height=%d", height) - time.Sleep(blockSleepDuration) + if height > prevHeight { + log.Debugf("Waiting for next block, current height %d (target height %d)", height, endHeight) + prevHeight = height + } + + if err = common.SleepWithContext(ctx, blockSleepDuration); err != nil { + return 0, err } } - - return 0, fmt.Errorf("timed out waiting for blocks") } // CreateWallet creates a monero wallet from a private keypair. @@ -56,10 +60,11 @@ func CreateWallet( env common.Environment, client WalletClient, kpAB *mcrypto.PrivateKeyPair, + restoreHeight uint64, ) (mcrypto.Address, error) { t := time.Now().Format(common.TimeFmtNSecs) walletName := fmt.Sprintf("%s-%s", name, t) - if err := client.GenerateFromKeys(kpAB, walletName, "", env); err != nil { + if err := client.GenerateFromKeys(kpAB, restoreHeight, walletName, "", env); err != nil { return "", err } diff --git a/monero/utils_test.go b/monero/utils_test.go index f09ab631..e8ea0750 100644 --- a/monero/utils_test.go +++ b/monero/utils_test.go @@ -1,6 +1,7 @@ package monero import ( + "context" "path" "testing" @@ -13,12 +14,12 @@ import ( func TestWaitForBlocks(t *testing.T) { c := CreateWalletClient(t) - heightBefore, err := c.GetHeight() + heightBefore, err := c.GetChainHeight() require.NoError(t, err) - heightAfter, err := WaitForBlocks(c, 1) + heightAfter, err := WaitForBlocks(context.Background(), c, 2) require.NoError(t, err) - require.GreaterOrEqual(t, heightAfter-heightBefore, uint64(1)) + require.GreaterOrEqual(t, heightAfter-heightBefore, uint64(2)) } func TestCreateMoneroWallet(t *testing.T) { @@ -31,7 +32,11 @@ func TestCreateMoneroWallet(t *testing.T) { MoneroWalletRPCPath: moneroWalletRPCPath, }) require.NoError(t, err) - addr, err := CreateWallet("create-wallet-test", common.Development, c, kp) + + height, err := c.GetHeight() + require.NoError(t, err) + + addr, err := CreateWallet("create-wallet-test", common.Development, c, kp, height) require.NoError(t, err) require.Equal(t, kp.Address(common.Development), addr) } diff --git a/monero/wallet_client.go b/monero/wallet_client.go index 0bfa2d2f..19c5570e 100644 --- a/monero/wallet_client.go +++ b/monero/wallet_client.go @@ -2,6 +2,7 @@ package monero import ( "bufio" + "context" "errors" "fmt" "net" @@ -14,6 +15,7 @@ import ( "time" "github.com/MarinX/monerorpc" + monerodaemon "github.com/MarinX/monerorpc/daemon" "github.com/MarinX/monerorpc/wallet" "github.com/athanorlabs/atomic-swap/common" @@ -22,6 +24,10 @@ import ( const ( moneroWalletRPCLogPrefix = "[monero-wallet-rpc]: " + + // MinSpendConfirmations is the number of confirmations required on transaction + // outputs before they can be spent again. + MinSpendConfirmations = 10 ) // WalletClient represents a monero-wallet-rpc client. @@ -30,12 +36,27 @@ type WalletClient interface { UnlockClient() GetAccounts() (*wallet.GetAccountsResponse, error) GetAddress(idx uint64) (*wallet.GetAddressResponse, error) + PrimaryWalletAddress() mcrypto.Address GetBalance(idx uint64) (*wallet.GetBalanceResponse, error) Transfer(to mcrypto.Address, accountIdx, amount uint64) (*wallet.TransferResponse, error) SweepAll(to mcrypto.Address, accountIdx uint64) (*wallet.SweepAllResponse, error) - GenerateFromKeys(kp *mcrypto.PrivateKeyPair, filename, password string, env common.Environment) error - GenerateViewOnlyWalletFromKeys(vk *mcrypto.PrivateViewKey, address mcrypto.Address, filename, password string) error + WaitForTransReceipt(req *WaitForReceiptRequest) (*wallet.Transfer, error) + GenerateFromKeys( + kp *mcrypto.PrivateKeyPair, + restoreHeight uint64, + filename, + password string, + env common.Environment, + ) error + GenerateViewOnlyWalletFromKeys( + vk *mcrypto.PrivateViewKey, + address mcrypto.Address, + restoreHeight uint64, + filename, + password string, + ) error GetHeight() (uint64, error) + GetChainHeight() (uint64, error) Refresh() error CreateWallet(filename, password string) error OpenWallet(filename, password string) error @@ -56,11 +77,24 @@ type WalletClientConf struct { LogPath string // optional, default is dir(WalletFilePath)/../monero-wallet-rpc.log } +// WaitForReceiptRequest wraps the input parameters for WaitForTransReceipt +type WaitForReceiptRequest struct { + Ctx context.Context + TxID string + DestAddr mcrypto.Address + NumConfirmations uint64 + AccountIdx uint64 +} + type walletClient struct { - mu sync.Mutex - rpc wallet.Wallet // full API with slightly different method signatures - endpoint string - rpcProcess *os.Process // monero-wallet-rpc process that we create + mu sync.Mutex + wRPC wallet.Wallet // full monero-wallet-rpc API (larger than the WalletClient interface) + dRPC monerodaemon.Daemon // full monerod RPC API + endpoint string + primaryWallet string // primary wallet name not including any directory + primaryWalletPassword string // password for the primary wallet + primaryWalletAddr mcrypto.Address + rpcProcess *os.Process // monero-wallet-rpc process that we create } // NewWalletClient returns a WalletClient for a newly created monero-wallet-rpc process. @@ -68,13 +102,6 @@ func NewWalletClient(conf *WalletClientConf) (WalletClient, error) { if conf.WalletFilePath == "" { panic("WalletFilePath is a required conf field") // should have been caught before we were invoked } - if conf.WalletPort == 0 { - var err error - conf.WalletPort, err = getFreePort() - if err != nil { - return nil, err - } - } if path.Dir(conf.WalletFilePath) == "." { return nil, errors.New("wallet file can not be in the current working directory") @@ -86,34 +113,44 @@ func NewWalletClient(conf *WalletClientConf) (WalletClient, error) { } isNewWallet := !walletExists - endpoint, proc, err := createWalletRPCService(conf) + proc, err := createWalletRPCService(conf) if err != nil { return nil, err } - c := NewThinWalletClient(endpoint).(*walletClient) + c := NewThinWalletClient(conf.MonerodHost, conf.MonerodPort, conf.WalletPort).(*walletClient) c.rpcProcess = proc - walletName := path.Base(conf.WalletFilePath) + c.primaryWallet = path.Base(conf.WalletFilePath) + c.primaryWalletPassword = conf.WalletPassword if isNewWallet { - if err := c.CreateWallet(walletName, conf.WalletPassword); err != nil { + if err = c.CreateWallet(c.primaryWallet, conf.WalletPassword); err != nil { c.Close() return nil, err } log.Infof("New Monero wallet %s created", conf.WalletFilePath) } - if err := c.OpenWallet(walletName, conf.WalletPassword); err != nil { + if err = c.OpenPrimaryWallet(); err != nil { c.Close() return nil, err } + acctResp, err := c.GetAddress(0) + if err != nil { + c.Close() + return nil, err + } + c.primaryWalletAddr = mcrypto.Address(acctResp.Address) return c, nil } // NewThinWalletClient returns a WalletClient for an existing monero-wallet-rpc process. -func NewThinWalletClient(endpoint string) WalletClient { +func NewThinWalletClient(monerodHost string, monerodPort uint, walletPort uint) WalletClient { + monerodEndpoint := fmt.Sprintf("http://%s:%d/json_rpc", monerodHost, monerodPort) + walletEndpoint := fmt.Sprintf("http://127.0.0.1:%d/json_rpc", walletPort) return &walletClient{ - rpc: monerorpc.New(endpoint, nil).Wallet, - endpoint: endpoint, + dRPC: monerorpc.New(monerodEndpoint, nil).Daemon, + wRPC: monerorpc.New(walletEndpoint, nil).Wallet, + endpoint: walletEndpoint, } } @@ -126,28 +163,72 @@ func (c *walletClient) UnlockClient() { } func (c *walletClient) GetAccounts() (*wallet.GetAccountsResponse, error) { - return c.rpc.GetAccounts(&wallet.GetAccountsRequest{}) + return c.wRPC.GetAccounts(&wallet.GetAccountsRequest{}) } func (c *walletClient) GetBalance(idx uint64) (*wallet.GetBalanceResponse, error) { - return c.rpc.GetBalance(&wallet.GetBalanceRequest{ + return c.wRPC.GetBalance(&wallet.GetBalanceRequest{ AccountIndex: idx, }) } +// WaitForTransReceipt waits for the passed monero transaction ID to receive +// numConfirmations and returns the transfer information. While this function will always +// wait for the transaction to leave the mem-pool even if zero confirmations are +// requested, it is the caller's responsibility to request enough confirmations that the +// returned transfer information will not be invalidated by a block reorg. +func (c *walletClient) WaitForTransReceipt(req *WaitForReceiptRequest) (*wallet.Transfer, error) { + height, err := c.GetHeight() + if err != nil { + return nil, err + } + + var transfer *wallet.Transfer + + for { + if err = c.Refresh(); err != nil { + return nil, err + } + transferResp, err := c.wRPC.GetTransferByTxid(&wallet.GetTransferByTxidRequest{ + TxID: req.TxID, + AccountIndex: req.AccountIdx, + }) + if err != nil { + return nil, err + } + + transfer = &transferResp.Transfer + log.Infof("Received %d of %d confirmations of XMR TXID=%s (height=%d)", + transfer.Confirmations, + req.NumConfirmations, + req.TxID, + height) + // wait for transaction be mined (height set) even if 0 confirmations requested + if transfer.Height > 0 && transfer.Confirmations >= req.NumConfirmations { + break + } + + height, err = WaitForBlocks(req.Ctx, c, 1) + if err != nil { + return nil, err + } + } + + return transfer, nil +} + func (c *walletClient) Transfer(to mcrypto.Address, accountIdx, amount uint64) (*wallet.TransferResponse, error) { - return c.rpc.Transfer(&wallet.TransferRequest{ + return c.wRPC.Transfer(&wallet.TransferRequest{ Destinations: []wallet.Destination{{ Amount: amount, Address: string(to), }}, AccountIndex: accountIdx, - Priority: 0, }) } func (c *walletClient) SweepAll(to mcrypto.Address, accountIdx uint64) (*wallet.SweepAllResponse, error) { - return c.rpc.SweepAll(&wallet.SweepAllRequest{ + return c.wRPC.SweepAll(&wallet.SweepAllRequest{ AccountIndex: accountIdx, Address: string(to), }) @@ -156,26 +237,29 @@ func (c *walletClient) SweepAll(to mcrypto.Address, accountIdx uint64) (*wallet. // GenerateFromKeys creates a wallet from a given wallet address, view key, and optional spend key func (c *walletClient) GenerateFromKeys( kp *mcrypto.PrivateKeyPair, + restoreHeight uint64, filename, password string, env common.Environment, ) error { - return c.generateFromKeys(kp.SpendKey(), kp.ViewKey(), kp.Address(env), filename, password) + return c.generateFromKeys(kp.SpendKey(), kp.ViewKey(), kp.Address(env), restoreHeight, filename, password) } // GenerateViewOnlyWalletFromKeys creates a view-only wallet from a given view key and address func (c *walletClient) GenerateViewOnlyWalletFromKeys( vk *mcrypto.PrivateViewKey, address mcrypto.Address, + restoreHeight uint64, filename, password string, ) error { - return c.generateFromKeys(nil, vk, address, filename, password) + return c.generateFromKeys(nil, vk, address, restoreHeight, filename, password) } func (c *walletClient) generateFromKeys( sk *mcrypto.PrivateSpendKey, vk *mcrypto.PrivateViewKey, address mcrypto.Address, + restoreHeight uint64, filename, password string, ) error { @@ -189,12 +273,13 @@ func (c *walletClient) generateFromKeys( spendKey = sk.Hex() } - res, err := c.rpc.GenerateFromKeys(&wallet.GenerateFromKeysRequest{ - Filename: filename, - Address: string(address), - Viewkey: vk.Hex(), - Spendkey: spendKey, - Password: password, + res, err := c.wRPC.GenerateFromKeys(&wallet.GenerateFromKeysRequest{ + Filename: filename, + Address: string(address), + RestoreHeight: restoreHeight, + Viewkey: vk.Hex(), + Spendkey: spendKey, + Password: password, }) if err != nil { return err @@ -212,18 +297,18 @@ func (c *walletClient) generateFromKeys( } func (c *walletClient) GetAddress(idx uint64) (*wallet.GetAddressResponse, error) { - return c.rpc.GetAddress(&wallet.GetAddressRequest{ + return c.wRPC.GetAddress(&wallet.GetAddressRequest{ AccountIndex: idx, }) } func (c *walletClient) Refresh() error { - _, err := c.rpc.Refresh(&wallet.RefreshRequest{}) + _, err := c.wRPC.Refresh(&wallet.RefreshRequest{}) return err } func (c *walletClient) CreateWallet(filename, password string) error { - return c.rpc.CreateWallet(&wallet.CreateWalletRequest{ + return c.wRPC.CreateWallet(&wallet.CreateWalletRequest{ Filename: filename, Password: password, Language: "English", @@ -231,24 +316,47 @@ func (c *walletClient) CreateWallet(filename, password string) error { } func (c *walletClient) OpenWallet(filename, password string) error { - return c.rpc.OpenWallet(&wallet.OpenWalletRequest{ + return c.wRPC.OpenWallet(&wallet.OpenWalletRequest{ Filename: filename, Password: password, }) } +func (c *walletClient) OpenPrimaryWallet() error { + return c.OpenWallet(c.primaryWallet, c.primaryWalletPassword) +} + +func (c *walletClient) PrimaryWalletAddress() mcrypto.Address { + if c.primaryWalletAddr == "" { + // Initialised in constructor function, so this shouldn't ever happen + panic("primary wallet address was not initialised") + } + return c.primaryWalletAddr +} + func (c *walletClient) CloseWallet() error { - return c.rpc.CloseWallet() + return c.wRPC.CloseWallet() } func (c *walletClient) GetHeight() (uint64, error) { - res, err := c.rpc.GetHeight() + res, err := c.wRPC.GetHeight() if err != nil { return 0, err } return res.Height, nil } +// GetChainHeight gets the blockchain height directly from the monero daemon instead +// of the wallet height. Unlike the wallet method GetHeight, this method does not +// require a wallet to be open and is safe to call without grabbing the client mutex. +func (c *walletClient) GetChainHeight() (uint64, error) { + res, err := c.dRPC.GetBlockCount() + if err != nil { + return 0, err + } + return res.Count, nil +} + func (c *walletClient) Endpoint() string { return c.endpoint } @@ -301,15 +409,16 @@ func validateMonerodConfig(env common.Environment, monerodHost string, monerodPo return nil } -// createWalletRPCService starts a monero-wallet-rpc listening on a random port for tests. The json_rpc -// URL of the started service is returned. -func createWalletRPCService(conf *WalletClientConf) (string, *os.Process, error) { +// createWalletRPCService starts a monero-wallet-rpc instance. Default values are assigned +// to the MonerodHost, MonerodPort, WalletPort and LogPath fields of the config if they +// are not already set. +func createWalletRPCService(conf *WalletClientConf) (*os.Process, error) { walletRPCBin := conf.MoneroWalletRPCPath if walletRPCBin == "" { var err error walletRPCBin, err = getMoneroWalletRPCBin() if err != nil { - return "", nil, err + return nil, err } } @@ -328,7 +437,7 @@ func createWalletRPCService(conf *WalletClientConf) (string, *os.Process, error) } if err := validateMonerodConfig(conf.Env, conf.MonerodHost, conf.MonerodPort); err != nil { - return "", nil, err + return nil, err } if conf.LogPath == "" { @@ -336,13 +445,21 @@ func createWalletRPCService(conf *WalletClientConf) (string, *os.Process, error) conf.LogPath = path.Join(path.Dir(path.Dir(conf.WalletFilePath)), "monero-wallet-rpc.log") } + if conf.WalletPort == 0 { + var err error + conf.WalletPort, err = getFreePort() + if err != nil { + return nil, err + } + } + walletRPCBinArgs := getWalletRPCFlags(conf) proc, err := launchMoneroWalletRPCChild(walletRPCBin, walletRPCBinArgs...) if err != nil { - return "", nil, fmt.Errorf("%w, see %s for details", err, conf.LogPath) + return nil, fmt.Errorf("%w, see %s for details", err, conf.LogPath) } - return fmt.Sprintf("http://127.0.0.1:%d/json_rpc", conf.WalletPort), proc, nil + return proc, nil } // getMoneroWalletRPCBin returns the monero-wallet-rpc binary. It first looks for @@ -351,9 +468,9 @@ func createWalletRPCService(conf *WalletClientConf) (string, *os.Process, error) func getMoneroWalletRPCBin() (string, error) { execName := "monero-wallet-rpc" priorityPath := path.Join("monero-bin", execName) - path, err := exec.LookPath(priorityPath) + execPath, err := exec.LookPath(priorityPath) if err == nil { - return path, nil + return execPath, nil } if !errors.Is(err, os.ErrNotExist) { return "", err diff --git a/monero/wallet_client_test.go b/monero/wallet_client_test.go index ce97c76f..f6363d20 100644 --- a/monero/wallet_client_test.go +++ b/monero/wallet_client_test.go @@ -1,13 +1,10 @@ package monero import ( - "crypto/rand" - "fmt" - "math/big" + "context" "os" "path" "testing" - "time" "github.com/stretchr/testify/require" @@ -18,14 +15,14 @@ import ( var moneroWalletRPCPath = path.Join("..", "monero-bin", "monero-wallet-rpc") func TestClient_Transfer(t *testing.T) { - const amount = 2800000000 + amount := common.MoneroToPiconero(10) // 1k monero + cXMRMaker := CreateWalletClient(t) - MineMinXMRBalance(t, cXMRMaker, amount) + MineMinXMRBalance(t, cXMRMaker, amount+common.MoneroToPiconero(0.1)) // add a little extra for fees balance := GetBalance(t, cXMRMaker) - t.Log("balance: ", balance.Balance) - t.Log("unlocked balance: ", balance.UnlockedBalance) - t.Log("blocks to unlock: ", balance.BlocksToUnlock) + t.Logf("Bob's initial balance: bal=%d unlocked=%d blocks-to-unlock=%d", + balance.Balance, balance.UnlockedBalance, balance.BlocksToUnlock) require.Greater(t, balance.UnlockedBalance, uint64(amount)) kpA, err := mcrypto.GenerateKeys() @@ -34,61 +31,121 @@ func TestClient_Transfer(t *testing.T) { kpB, err := mcrypto.GenerateKeys() require.NoError(t, err) - kpABPub := mcrypto.SumSpendAndViewKeys(kpA.PublicKeyPair(), kpB.PublicKeyPair()) + abAddress := mcrypto.SumSpendAndViewKeys(kpA.PublicKeyPair(), kpB.PublicKeyPair()).Address(common.Mainnet) vkABPriv := mcrypto.SumPrivateViewKeys(kpA.ViewKey(), kpB.ViewKey()) + // Transfer from Bob's account to the Alice+Bob swap account + transResp, err := cXMRMaker.Transfer(abAddress, 0, uint64(amount)) + require.NoError(t, err) + t.Logf("Bob sent %f (+fee %f) XMR to A+B address with TX ID %s", + common.MoneroAmount(transResp.Amount).AsMonero(), common.MoneroAmount(transResp.Fee).AsMonero(), + transResp.TxHash) + require.NoError(t, err) + transfer, err := cXMRMaker.WaitForTransReceipt(&WaitForReceiptRequest{ + Ctx: context.Background(), + TxID: transResp.TxHash, + DestAddr: abAddress, + NumConfirmations: MinSpendConfirmations, + AccountIdx: 0, + }) + require.NoError(t, err) + require.GreaterOrEqual(t, transfer.Confirmations, uint64(MinSpendConfirmations)) + t.Logf("Bob's TX was mined at height %d with %d confirmations", transfer.Height, transfer.Confirmations) + cXMRMaker.Close() // Done with bob, make sure no one uses him again + + // Establish Alice's primary wallet + alicePrimaryWallet := "test-swap-wallet" cXMRTaker, err := NewWalletClient(&WalletClientConf{ Env: common.Development, - WalletFilePath: path.Join(t.TempDir(), "wallet", "not-used"), + WalletFilePath: path.Join(t.TempDir(), "wallet", alicePrimaryWallet), MoneroWalletRPCPath: moneroWalletRPCPath, }) require.NoError(t, err) - require.NoError(t, cXMRTaker.CloseWallet()) + addrResp, err := cXMRTaker.GetAddress(0) + require.NoError(t, err) + alicePrimaryAddr := mcrypto.Address(addrResp.Address) - // generate view-only account for A+B + // Alice generates a view-only wallet for A+B to confirm that Bob sent the funds viewWalletName := "test-view-wallet" - err = cXMRTaker.(*walletClient).generateFromKeys(nil, vkABPriv, kpABPub.Address(common.Mainnet), viewWalletName, "") - require.NoError(t, err) - err = cXMRTaker.OpenWallet(viewWalletName, "") + err = cXMRTaker.GenerateViewOnlyWalletFromKeys(vkABPriv, abAddress, transfer.Height, viewWalletName, "") require.NoError(t, err) - // transfer to account A+B - resp, err := cXMRMaker.Transfer(kpABPub.Address(common.Mainnet), 0, amount) + // Verify that generateFromKeys closed Alice's primary wallet and opened the new A+B + // view wallet by checking the address of the current wallet + addrResp, err = cXMRTaker.GetAddress(0) require.NoError(t, err) - t.Logf("Transfer resp: %#v", resp) - _, err = WaitForBlocks(cXMRMaker, 1) + require.Equal(t, abAddress, mcrypto.Address(addrResp.Address)) + + balance = GetBalance(t, cXMRTaker) + height, err := cXMRTaker.GetHeight() require.NoError(t, err) + t.Logf("A+B View-Only wallet balance: bal=%d unlocked=%d blocks-to-unlock=%d, cur-height=%d", + balance.Balance, balance.UnlockedBalance, balance.BlocksToUnlock, height) + require.Zero(t, balance.BlocksToUnlock) + require.Equal(t, balance.UnlockedBalance, balance.Balance) + require.Equal(t, balance.UnlockedBalance, uint64(amount)) - // Something strange is happening below. On the first loop iteration, we are seeing a positive - // Balance, a zero UnlockedBalance, but BlocksToUnlock is also zero. :| One the second loop, - // BlocksToUnlock is above zero. - for { - t.Log("checking XMR Taker balance:") - balance = GetBalance(t, cXMRTaker) - t.Log("\tbalance of AB: ", balance.Balance) - t.Log("\tunlocked balance of AB: ", balance.UnlockedBalance) - t.Log("\tblocks to unlock AB: ", balance.BlocksToUnlock) - if balance.UnlockedBalance > 0 { - require.NoError(t, cXMRTaker.CloseWallet()) - break - } - time.Sleep(backgroundMineInterval) - } - - // generate spend account for A+B + // At this point Alice has received the key from Bob to create an A+B spend wallet. + // She'll now sweep the funds from the A+B spend wallet into her primary wallet. spendWalletName := "test-spend-wallet" + // TODO: Can we convert View-only wallet into spend wallet if it is the same wallet? skAKPriv := mcrypto.SumPrivateSpendKeys(kpA.SpendKey(), kpB.SpendKey()) - err = cXMRTaker.(*walletClient).generateFromKeys(skAKPriv, vkABPriv, kpABPub.Address(common.Mainnet), spendWalletName, "") //nolint:lll + err = cXMRTaker.(*walletClient).generateFromKeys(skAKPriv, vkABPriv, abAddress, transfer.Height, spendWalletName, "") require.NoError(t, err) balance = GetBalance(t, cXMRTaker) - require.Greater(t, balance.UnlockedBalance, uint64(0)) + // Verify that the spend wallet, like the view-only wallet, has the exact amount expected in it + require.Equal(t, balance.UnlockedBalance, uint64(amount)) - // transfer from account A+B back to XMRMaker's address - xmrmakerAddr, err := cXMRTaker.GetAddress(0) + // Alice transfers from A+B spend wallet to her primary wallet's address + sweepResp, err := cXMRTaker.SweepAll(alicePrimaryAddr, 0) require.NoError(t, err) - _, err = cXMRTaker.Transfer(mcrypto.Address(xmrmakerAddr.Address), 0, 1) + t.Logf("%#v", sweepResp) + require.Len(t, sweepResp.TxHashList, 1) // In our case, it should always be a single transaction + sweepTxID := sweepResp.TxHashList[0] + sweepAmount := sweepResp.AmountList[0] + sweepFee := sweepResp.FeeList[0] + + t.Logf("Sweep of A+B wallet sent %d with fees %d to Alice's primary wallet", + sweepAmount, sweepFee) + require.Equal(t, uint64(amount), sweepAmount+sweepFee) + + transfer, err = cXMRTaker.WaitForTransReceipt(&WaitForReceiptRequest{ + Ctx: context.Background(), + TxID: sweepTxID, + DestAddr: alicePrimaryAddr, + NumConfirmations: 2, + AccountIdx: 0, + }) require.NoError(t, err) + require.Equal(t, sweepFee, transfer.Fee) + require.Equal(t, sweepAmount, transfer.Amount) + t.Logf("Alice's sweep transactions was mined at height %d with %d confirmations", + transfer.Height, transfer.Confirmations) + + // Verify zero balance of A+B wallet after sweep + balance = GetBalance(t, cXMRTaker) + require.Equal(t, balance.Balance, uint64(0)) + + // Switch Alice back to her primary wallet + require.NoError(t, cXMRTaker.OpenWallet(alicePrimaryWallet, "")) + + balance = GetBalance(t, cXMRTaker) + t.Logf("Alice's primary wallet after sweep: bal=%d unlocked=%d blocks-to-unlock=%d", + balance.Balance, balance.UnlockedBalance, balance.BlocksToUnlock) + require.Equal(t, balance.Balance, sweepAmount) +} + +func Test_walletClient_SweepAll_nothingToSweepReturnsError(t *testing.T) { + emptyWallet := CreateWalletClient(t) + takerWallet := CreateWalletClient(t) + + addrResp, err := takerWallet.GetAddress(0) + require.NoError(t, err) + destAddr := mcrypto.Address(addrResp.Address) + + _, err = emptyWallet.SweepAll(destAddr, 0) + require.ErrorContains(t, err, "No unlocked balance in the specified account") } func TestClient_CloseWallet(t *testing.T) { @@ -130,18 +187,20 @@ func TestClient_GetHeight(t *testing.T) { }) require.NoError(t, err) defer c.Close() - resp, err := c.GetHeight() + + require.NoError(t, c.Refresh()) + walletHeight, err := c.GetHeight() require.NoError(t, err) - require.NotEqual(t, 0, resp) + chainHeight, err := c.GetChainHeight() + require.NoError(t, err) + require.GreaterOrEqual(t, chainHeight, walletHeight) + require.LessOrEqual(t, chainHeight-walletHeight, uint64(2)) } func TestCallGenerateFromKeys(t *testing.T) { kp, err := mcrypto.GenerateKeys() require.NoError(t, err) - r, err := rand.Int(rand.Reader, big.NewInt(999)) - require.NoError(t, err) - c, err := NewWalletClient(&WalletClientConf{ Env: common.Development, WalletFilePath: path.Join(t.TempDir(), "wallet", "not-used"), @@ -150,13 +209,22 @@ func TestCallGenerateFromKeys(t *testing.T) { require.NoError(t, err) defer c.Close() + height, err := c.GetHeight() + require.NoError(t, err) + addr, err := c.GetAddress(0) require.NoError(t, err) t.Logf("Address %s", addr.Address) // initial wallet automatically closed when a new wallet is opened - err = c.(*walletClient).generateFromKeys(kp.SpendKey(), kp.ViewKey(), kp.Address(common.Mainnet), - fmt.Sprintf("test-wallet-%d", r), "") + err = c.(*walletClient).generateFromKeys( + kp.SpendKey(), + kp.ViewKey(), + kp.Address(common.Mainnet), + height, + "swap-deposit-wallet", + "", + ) require.NoError(t, err) addr, err = c.GetAddress(0) @@ -206,3 +274,26 @@ func Test_validateMonerodConfig_invalidPort(t *testing.T) { require.Error(t, err) require.Contains(t, err.Error(), "connection refused") } + +func Test_walletClient_waitForConfirmations_contextCancelled(t *testing.T) { + amount := common.MoneroToPiconero(10) // 1k monero + destAddr := mcrypto.Address(blockRewardAddress) + + c := CreateWalletClient(t) + MineMinXMRBalance(t, c, amount+common.MoneroToPiconero(0.1)) // add a little extra for fees + + transResp, err := c.Transfer(destAddr, 0, uint64(amount)) + require.NoError(t, err) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err = c.WaitForTransReceipt(&WaitForReceiptRequest{ + Ctx: ctx, + TxID: transResp.TxHash, + DestAddr: destAddr, + NumConfirmations: 999999999, // wait for a number of confirmations that would take a long time + AccountIdx: 0, + }) + require.ErrorIs(t, err, context.Canceled) +} diff --git a/net/message/message.go b/net/message/message.go index 28433f75..f6d197bf 100644 --- a/net/message/message.go +++ b/net/message/message.go @@ -6,9 +6,9 @@ import ( "fmt" "math/big" - "github.com/athanorlabs/atomic-swap/common/types" - ethcommon "github.com/ethereum/go-ethereum/common" + + "github.com/athanorlabs/atomic-swap/common/types" ) // Type represents the type of a network message @@ -227,7 +227,8 @@ func (m *NotifyETHLocked) Type() Type { // NotifyXMRLock is sent by XMRMaker to XMRTaker after locking his XMR. type NotifyXMRLock struct { - Address string + Address string // address the monero was sent to + TxID string // Monero transaction ID (transaction hash in hex) } // String ... diff --git a/protocol/xmrmaker/message_handler.go b/protocol/xmrmaker/message_handler.go index 6e59518e..8398dd94 100644 --- a/protocol/xmrmaker/message_handler.go +++ b/protocol/xmrmaker/message_handler.go @@ -173,19 +173,15 @@ func (s *swapState) handleNotifyETHLocked(msg *message.NotifyETHLocked) (net.Mes // TODO: check these (in checkContract) (#161) s.setTimeouts(msg.ContractSwap.Timeout0, msg.ContractSwap.Timeout1) - addrAB, err := s.lockFunds(common.MoneroToPiconero(s.info.ProvidedAmount())) + notifyXMRLocked, err := s.lockFunds(common.MoneroToPiconero(s.info.ProvidedAmount())) if err != nil { return nil, fmt.Errorf("failed to lock funds: %w", err) } - out := &message.NotifyXMRLock{ - Address: string(addrAB), - } - go s.runT0ExpirationHandler() s.setNextExpectedMessage(&message.NotifyReady{}) - return out, nil + return notifyXMRLocked, nil } func (s *swapState) runT0ExpirationHandler() { diff --git a/protocol/xmrmaker/mock_backend_test.go b/protocol/xmrmaker/mock_backend_test.go index 1155a316..8121bbe6 100644 --- a/protocol/xmrmaker/mock_backend_test.go +++ b/protocol/xmrmaker/mock_backend_test.go @@ -16,6 +16,7 @@ import ( types "github.com/athanorlabs/atomic-swap/common/types" mcrypto "github.com/athanorlabs/atomic-swap/crypto/monero" contracts "github.com/athanorlabs/atomic-swap/ethereum" + monero "github.com/athanorlabs/atomic-swap/monero" net "github.com/athanorlabs/atomic-swap/net" message "github.com/athanorlabs/atomic-swap/net/message" swap "github.com/athanorlabs/atomic-swap/protocol/swap" @@ -323,31 +324,31 @@ func (mr *MockBackendMockRecorder) FilterLogs(arg0, arg1 interface{}) *gomock.Ca } // GenerateFromKeys mocks base method. -func (m *MockBackend) GenerateFromKeys(arg0 *mcrypto.PrivateKeyPair, arg1, arg2 string, arg3 common.Environment) error { +func (m *MockBackend) GenerateFromKeys(arg0 *mcrypto.PrivateKeyPair, arg1 uint64, arg2, arg3 string, arg4 common.Environment) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GenerateFromKeys", arg0, arg1, arg2, arg3) + ret := m.ctrl.Call(m, "GenerateFromKeys", arg0, arg1, arg2, arg3, arg4) ret0, _ := ret[0].(error) return ret0 } // GenerateFromKeys indicates an expected call of GenerateFromKeys. -func (mr *MockBackendMockRecorder) GenerateFromKeys(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +func (mr *MockBackendMockRecorder) GenerateFromKeys(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateFromKeys", reflect.TypeOf((*MockBackend)(nil).GenerateFromKeys), arg0, arg1, arg2, arg3) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateFromKeys", reflect.TypeOf((*MockBackend)(nil).GenerateFromKeys), arg0, arg1, arg2, arg3, arg4) } // GenerateViewOnlyWalletFromKeys mocks base method. -func (m *MockBackend) GenerateViewOnlyWalletFromKeys(arg0 *mcrypto.PrivateViewKey, arg1 mcrypto.Address, arg2, arg3 string) error { +func (m *MockBackend) GenerateViewOnlyWalletFromKeys(arg0 *mcrypto.PrivateViewKey, arg1 mcrypto.Address, arg2 uint64, arg3, arg4 string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GenerateViewOnlyWalletFromKeys", arg0, arg1, arg2, arg3) + ret := m.ctrl.Call(m, "GenerateViewOnlyWalletFromKeys", arg0, arg1, arg2, arg3, arg4) ret0, _ := ret[0].(error) return ret0 } // GenerateViewOnlyWalletFromKeys indicates an expected call of GenerateViewOnlyWalletFromKeys. -func (mr *MockBackendMockRecorder) GenerateViewOnlyWalletFromKeys(arg0, arg1, arg2, arg3 interface{}) *gomock.Call { +func (mr *MockBackendMockRecorder) GenerateViewOnlyWalletFromKeys(arg0, arg1, arg2, arg3, arg4 interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateViewOnlyWalletFromKeys", reflect.TypeOf((*MockBackend)(nil).GenerateViewOnlyWalletFromKeys), arg0, arg1, arg2, arg3) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GenerateViewOnlyWalletFromKeys", reflect.TypeOf((*MockBackend)(nil).GenerateViewOnlyWalletFromKeys), arg0, arg1, arg2, arg3, arg4) } // GetAccounts mocks base method. @@ -395,6 +396,21 @@ func (mr *MockBackendMockRecorder) GetBalance(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetBalance", reflect.TypeOf((*MockBackend)(nil).GetBalance), arg0) } +// GetChainHeight mocks base method. +func (m *MockBackend) GetChainHeight() (uint64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetChainHeight") + ret0, _ := ret[0].(uint64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetChainHeight indicates an expected call of GetChainHeight. +func (mr *MockBackendMockRecorder) GetChainHeight() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetChainHeight", reflect.TypeOf((*MockBackend)(nil).GetChainHeight)) +} + // GetHeight mocks base method. func (m *MockBackend) GetHeight() (uint64, error) { m.ctrl.T.Helper() @@ -509,6 +525,20 @@ func (mr *MockBackendMockRecorder) OpenWallet(arg0, arg1 interface{}) *gomock.Ca return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "OpenWallet", reflect.TypeOf((*MockBackend)(nil).OpenWallet), arg0, arg1) } +// PrimaryWalletAddress mocks base method. +func (m *MockBackend) PrimaryWalletAddress() mcrypto.Address { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "PrimaryWalletAddress") + ret0, _ := ret[0].(mcrypto.Address) + return ret0 +} + +// PrimaryWalletAddress indicates an expected call of PrimaryWalletAddress. +func (mr *MockBackendMockRecorder) PrimaryWalletAddress() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "PrimaryWalletAddress", reflect.TypeOf((*MockBackend)(nil).PrimaryWalletAddress)) +} + // Refresh mocks base method. func (m *MockBackend) Refresh() error { m.ctrl.T.Helper() @@ -549,30 +579,6 @@ func (mr *MockBackendMockRecorder) SetBaseXMRDepositAddress(arg0 interface{}) *g return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetBaseXMRDepositAddress", reflect.TypeOf((*MockBackend)(nil).SetBaseXMRDepositAddress), arg0) } -// SetContract mocks base method. -func (m *MockBackend) SetContract(arg0 *contracts.SwapFactory) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetContract", arg0) -} - -// SetContract indicates an expected call of SetContract. -func (mr *MockBackendMockRecorder) SetContract(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetContract", reflect.TypeOf((*MockBackend)(nil).SetContract), arg0) -} - -// SetContractAddress mocks base method. -func (m *MockBackend) SetContractAddress(arg0 common0.Address) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetContractAddress", arg0) -} - -// SetContractAddress indicates an expected call of SetContractAddress. -func (mr *MockBackendMockRecorder) SetContractAddress(arg0 interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetContractAddress", reflect.TypeOf((*MockBackend)(nil).SetContractAddress), arg0) -} - // SetEthAddress mocks base method. func (m *MockBackend) SetEthAddress(arg0 common0.Address) { m.ctrl.T.Helper() @@ -766,6 +772,21 @@ func (mr *MockBackendMockRecorder) WaitForTimestamp(arg0, arg1 interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitForTimestamp", reflect.TypeOf((*MockBackend)(nil).WaitForTimestamp), arg0, arg1) } +// WaitForTransReceipt mocks base method. +func (m *MockBackend) WaitForTransReceipt(arg0 *monero.WaitForReceiptRequest) (*wallet.Transfer, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "WaitForTransReceipt", arg0) + ret0, _ := ret[0].(*wallet.Transfer) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// WaitForTransReceipt indicates an expected call of WaitForTransReceipt. +func (mr *MockBackendMockRecorder) WaitForTransReceipt(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "WaitForTransReceipt", reflect.TypeOf((*MockBackend)(nil).WaitForTransReceipt), arg0) +} + // XMRDepositAddress mocks base method. func (m *MockBackend) XMRDepositAddress(arg0 *types.Hash) (mcrypto.Address, error) { m.ctrl.T.Helper() diff --git a/protocol/xmrmaker/recovery.go b/protocol/xmrmaker/recovery.go index cdc88b3d..8a23ef6c 100644 --- a/protocol/xmrmaker/recovery.go +++ b/protocol/xmrmaker/recovery.go @@ -53,6 +53,7 @@ func NewRecoveryState(b backend.Backend, dataDir string, secret *mcrypto.Private offerExtra: &types.OfferExtra{ InfoFile: pcommon.GetSwapRecoveryFilepath(dataDir), }, + walletScanHeight: 0, // could optimise this if we start recording it in the swap recovery info } if err := s.setContract(contractAddr); err != nil { diff --git a/protocol/xmrmaker/recovery_test.go b/protocol/xmrmaker/recovery_test.go index 77ae11d3..0f52f8d9 100644 --- a/protocol/xmrmaker/recovery_test.go +++ b/protocol/xmrmaker/recovery_test.go @@ -58,7 +58,7 @@ func TestClaimOrRecover_Recover(t *testing.T) { // lock XMR rs.ss.setXMRTakerPublicKeys(rs.ss.pubkeys, nil) - addrAB, err := rs.ss.lockFunds(1) + lockedXMR, err := rs.ss.lockFunds(1) require.NoError(t, err) // call refund w/ XMRTaker's spend key @@ -71,5 +71,6 @@ func TestClaimOrRecover_Recover(t *testing.T) { res, err := rs.ClaimOrRecover() require.NoError(t, err) require.True(t, res.Recovered) - require.Equal(t, addrAB, res.MoneroAddress) + require.Equal(t, lockedXMR.Address, string(res.MoneroAddress)) + require.NotEmpty(t, lockedXMR.Address, "") } diff --git a/protocol/xmrmaker/swap_state.go b/protocol/xmrmaker/swap_state.go index c9bf52ac..a1a2647e 100644 --- a/protocol/xmrmaker/swap_state.go +++ b/protocol/xmrmaker/swap_state.go @@ -64,6 +64,7 @@ type swapState struct { // XMRTaker's keys for this session xmrtakerPublicKeys *mcrypto.PublicKeyPair xmrtakerSecp256K1PublicKey *secp256k1.PublicKey + walletScanHeight uint64 // height of the monero blockchain when the swap is started // next expected network message nextExpectedMessage net.Message @@ -116,6 +117,15 @@ func newSwapState( } } + walletScanHeight, err := b.GetChainHeight() + if err != nil { + return nil, err + } + // reduce the scan height a little in case there is a block reorg + if walletScanHeight >= monero.MinSpendConfirmations { + walletScanHeight -= monero.MinSpendConfirmations + } + ctx, cancel := context.WithCancel(b.Ctx()) s := &swapState{ ctx: ctx, @@ -125,6 +135,7 @@ func newSwapState( offer: offer, offerExtra: offerExtra, offerManager: om, + walletScanHeight: walletScanHeight, nextExpectedMessage: &net.SendKeysMessage{}, readyCh: make(chan struct{}), info: info, @@ -304,7 +315,7 @@ func (s *swapState) reclaimMonero(skA *mcrypto.PrivateSpendKey) (mcrypto.Address s.LockClient() defer s.UnlockClient() - return monero.CreateWallet("xmrmaker-swap-wallet", s.Env(), s, kpAB) + return monero.CreateWallet("xmrmaker-swap-wallet", s.Env(), s, kpAB, s.walletScanHeight) } func (s *swapState) filterForRefund() (*mcrypto.PrivateSpendKey, error) { @@ -487,8 +498,8 @@ func (s *swapState) checkContract(txHash ethcommon.Hash) error { // lockFunds locks XMRMaker's funds in the monero account specified by public key // (S_a + S_b), viewable with (V_a + V_b) // It accepts the amount to lock as the input -func (s *swapState) lockFunds(amount common.MoneroAmount) (mcrypto.Address, error) { - kp := mcrypto.SumSpendAndViewKeys(s.xmrtakerPublicKeys, s.pubkeys) +func (s *swapState) lockFunds(amount common.MoneroAmount) (*message.NotifyXMRLock, error) { + swapDestAddr := mcrypto.SumSpendAndViewKeys(s.xmrtakerPublicKeys, s.pubkeys).Address(s.Env()) log.Infof("going to lock XMR funds, amount(piconero)=%d", amount) s.LockClient() @@ -496,31 +507,38 @@ func (s *swapState) lockFunds(amount common.MoneroAmount) (mcrypto.Address, erro balance, err := s.GetBalance(0) if err != nil { - return "", err + return nil, err } log.Debug("total XMR balance: ", balance.Balance) log.Info("unlocked XMR balance: ", balance.UnlockedBalance) - address := kp.Address(s.Env()) - txResp, err := s.Transfer(address, 0, uint64(amount)) + transResp, err := s.Transfer(swapDestAddr, 0, uint64(amount)) if err != nil { - return "", err + return nil, err } - log.Infof("locked XMR, txHash=%s fee=%d", txResp.TxHash, txResp.Fee) + log.Infof("locked %f XMR, txID=%s fee=%d", amount.AsMonero(), transResp.TxHash, transResp.Fee) - // wait for a new block - height, err := monero.WaitForBlocks(s, 1) + // TODO: It would be friendlier to concurrent swaps if we didn't hold the client lock + // for the entire confirmation period. Options to improve this include creating a + // separate monero-wallet-rpc instance for A+B wallets or carefully releasing the + // lock between confirmations and re-opening the A+B wallet after grabbing the + // lock again. + transfer, err := s.WaitForTransReceipt(&monero.WaitForReceiptRequest{ + Ctx: s.ctx, + TxID: transResp.TxHash, + DestAddr: swapDestAddr, + NumConfirmations: monero.MinSpendConfirmations, + AccountIdx: 0, + }) if err != nil { - return "", err + return nil, err } - log.Infof("monero block height: %d", height) - - if err := s.Refresh(); err != nil { - return "", err - } - - log.Infof("successfully locked XMR funds: address=%s", address) - return address, nil + log.Infof("Successfully locked XMR funds: txID=%s address=%s block=%d", + transfer.TxID, swapDestAddr, transfer.Height) + return &message.NotifyXMRLock{ + Address: string(swapDestAddr), + TxID: transfer.TxID, + }, nil } diff --git a/protocol/xmrmaker/swap_state_test.go b/protocol/xmrmaker/swap_state_test.go index 37fd2814..d7832e12 100644 --- a/protocol/xmrmaker/swap_state_test.go +++ b/protocol/xmrmaker/swap_state_test.go @@ -370,7 +370,7 @@ func TestSwapState_handleRefund(t *testing.T) { newSwap(t, s, [32]byte{}, refundKey, desiredAmount.BigInt(), duration) // lock XMR - addrAB, err := s.lockFunds(common.MoneroToPiconero(s.info.ProvidedAmount())) + lockedXMR, err := s.lockFunds(common.MoneroToPiconero(s.info.ProvidedAmount())) require.NoError(t, err) // call refund w/ XMRTaker's spend key @@ -386,7 +386,7 @@ func TestSwapState_handleRefund(t *testing.T) { addr, err := s.handleRefund(tx.Hash().String()) require.NoError(t, err) - require.Equal(t, addrAB, addr) + require.Equal(t, lockedXMR.Address, string(addr)) } func TestSwapState_HandleProtocolMessage_NotifyRefund(t *testing.T) { diff --git a/protocol/xmrtaker/instance.go b/protocol/xmrtaker/instance.go index 7bad5a09..60fac235 100644 --- a/protocol/xmrtaker/instance.go +++ b/protocol/xmrtaker/instance.go @@ -1,23 +1,15 @@ package xmrtaker import ( - "fmt" "sync" ethcommon "github.com/ethereum/go-ethereum/common" + logging "github.com/ipfs/go-log" "github.com/athanorlabs/atomic-swap/common" "github.com/athanorlabs/atomic-swap/common/types" - mcrypto "github.com/athanorlabs/atomic-swap/crypto/monero" - "github.com/athanorlabs/atomic-swap/monero" "github.com/athanorlabs/atomic-swap/protocol/backend" "github.com/athanorlabs/atomic-swap/protocol/txsender" - - logging "github.com/ipfs/go-log" -) - -const ( - swapDepositWallet = "swap-deposit-wallet" ) var ( @@ -30,8 +22,7 @@ type Instance struct { backend backend.Backend dataDir string - walletFile, walletPassword string - transferBack bool // transfer xmr back to original account + transferBack bool // transfer xmr back to original account // non-nil if a swap is currently happening, nil otherwise // map of offer IDs -> ongoing swaps @@ -41,70 +32,28 @@ type Instance struct { // Config contains the configuration values for a new XMRTaker instance. type Config struct { - Backend backend.Backend - DataDir string - MoneroWalletFile, MoneroWalletPassword string - TransferBack bool - ExternalSender bool + Backend backend.Backend + DataDir string + TransferBack bool + ExternalSender bool } // NewInstance returns a new instance of XMRTaker. // It accepts an endpoint to a monero-wallet-rpc instance where XMRTaker will generate // the account in which the XMR will be deposited. func NewInstance(cfg *Config) (*Instance, error) { - var ( - address mcrypto.Address - err error - ) - // if this is set, it transfers all xmr received during swaps back to the given wallet. if cfg.TransferBack { - address, err = getAddress(cfg.Backend, cfg.MoneroWalletFile, cfg.MoneroWalletPassword) - if err != nil { - return nil, err - } - cfg.Backend.SetBaseXMRDepositAddress(address) + cfg.Backend.SetBaseXMRDepositAddress(cfg.Backend.PrimaryWalletAddress()) } return &Instance{ - backend: cfg.Backend, - dataDir: cfg.DataDir, - walletFile: cfg.MoneroWalletFile, - walletPassword: cfg.MoneroWalletPassword, - swapStates: make(map[types.Hash]*swapState), + backend: cfg.Backend, + dataDir: cfg.DataDir, + swapStates: make(map[types.Hash]*swapState), }, nil } -func getAddress(walletClient monero.WalletClient, file, password string) (mcrypto.Address, error) { - // open XMR wallet, if it exists - if file != "" { - if err := walletClient.OpenWallet(file, password); err != nil { - return "", err - } - } else { - log.Info("monero wallet file not set; creating wallet swap-deposit-wallet") - err := walletClient.CreateWallet(swapDepositWallet, "") - if err != nil { - if err := walletClient.OpenWallet(swapDepositWallet, ""); err != nil { - return "", fmt.Errorf("failed to create or open swap deposit wallet: %w", err) - } - } - } - - // get wallet address to deposit funds into at end of swap - address, err := walletClient.GetAddress(0) - if err != nil { - return "", fmt.Errorf("failed to get monero wallet address: %w", err) - } - - err = walletClient.CloseWallet() - if err != nil { - return "", fmt.Errorf("failed to close wallet: %w", err) - } - - return mcrypto.Address(address.Address), nil -} - // Refund is called by the RPC function swap_refund. // If it's possible to refund the ongoing swap, it does that, then notifies the counterparty. func (a *Instance) Refund(offerID types.Hash) (ethcommon.Hash, error) { diff --git a/protocol/xmrtaker/instance_test.go b/protocol/xmrtaker/instance_test.go index 7a342535..9a31741f 100644 --- a/protocol/xmrtaker/instance_test.go +++ b/protocol/xmrtaker/instance_test.go @@ -3,17 +3,22 @@ package xmrtaker import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/athanorlabs/atomic-swap/monero" + "github.com/athanorlabs/atomic-swap/common/types" ) -func TestGetAddress(t *testing.T) { - c := monero.CreateWalletClient(t) - addr, err := getAddress(c, "", "") +func TestNewInstance(t *testing.T) { + i, err := NewInstance(&Config{ + Backend: newBackend(t), + DataDir: "", + TransferBack: true, + ExternalSender: false, + }) require.NoError(t, err) - - addr2, err := getAddress(c, swapDepositWallet, "") - require.NoError(t, err) - require.Equal(t, addr, addr2) + assert.Nil(t, i.GetOngoingSwapState(types.EmptyHash)) + assert.Equal(t, i.Provides(), types.ProvidesETH) + _, err = i.Refund(types.EmptyHash) + assert.ErrorIs(t, err, errNoOngoingSwap) } diff --git a/protocol/xmrtaker/message_handler.go b/protocol/xmrtaker/message_handler.go index 30680057..3e369fbd 100644 --- a/protocol/xmrtaker/message_handler.go +++ b/protocol/xmrtaker/message_handler.go @@ -6,13 +6,10 @@ import ( "strings" "time" - "github.com/MarinX/monerorpc/wallet" - "github.com/athanorlabs/atomic-swap/common" "github.com/athanorlabs/atomic-swap/common/types" mcrypto "github.com/athanorlabs/atomic-swap/crypto/monero" contracts "github.com/athanorlabs/atomic-swap/ethereum" - "github.com/athanorlabs/atomic-swap/monero" "github.com/athanorlabs/atomic-swap/net" "github.com/athanorlabs/atomic-swap/net/message" pcommon "github.com/athanorlabs/atomic-swap/protocol" @@ -228,9 +225,9 @@ func (s *swapState) handleNotifyXMRLock(msg *message.NotifyXMRLock) (net.Message // check that XMR was locked in expected account, and confirm amount vk := mcrypto.SumPrivateViewKeys(s.xmrmakerPrivateViewKey, s.privkeys.ViewKey()) sk := mcrypto.SumPublicKeys(s.xmrmakerPublicSpendKey, s.pubkeys.SpendKey()) - kp := mcrypto.NewPublicKeyPair(sk, vk.Public()) + lockedAddr := mcrypto.NewPublicKeyPair(sk, vk.Public()).Address(s.Env()) - if msg.Address != string(kp.Address(s.Env())) { + if msg.Address != string(lockedAddr) { return nil, fmt.Errorf("address received in message does not match expected address") } @@ -239,68 +236,39 @@ func (s *swapState) handleNotifyXMRLock(msg *message.NotifyXMRLock) (net.Message t := time.Now().Format(common.TimeFmtNSecs) walletName := fmt.Sprintf("xmrtaker-viewonly-wallet-%s", t) - if err := s.GenerateViewOnlyWalletFromKeys(vk, kp.Address(s.Env()), walletName, ""); err != nil { + if err := s.GenerateViewOnlyWalletFromKeys(vk, lockedAddr, s.walletScanHeight, walletName, ""); err != nil { return nil, fmt.Errorf("failed to generate view-only wallet to verify locked XMR: %w", err) } log.Debugf("generated view-only wallet to check funds: %s", walletName) - if s.Env() != common.Development { - log.Infof("waiting for new blocks...") - // wait for 2 new blocks, otherwise balance might be 0 - // TODO: check transaction hash (#164) - height, err := monero.WaitForBlocks(s.Backend, 2) - if err != nil { - return nil, err - } - - log.Infof("monero block height: %d", height) - } - - log.Debug("refreshing client...") - if err := s.Refresh(); err != nil { return nil, fmt.Errorf("failed to refresh client: %w", err) } - accounts, err := s.GetAccounts() + balance, err := s.GetBalance(0) if err != nil { return nil, fmt.Errorf("failed to get accounts: %w", err) } - var ( - balance *wallet.GetBalanceResponse - ) - - for i, acc := range accounts.SubaddressAccounts { - addr := acc.BaseAddress - - if mcrypto.Address(addr) == kp.Address(s.Env()) { - balance, err = s.GetBalance(uint64(i)) - if err != nil { - return nil, fmt.Errorf("failed to get balance: %w", err) - } - - break - } - } - - if balance == nil { - return nil, fmt.Errorf("failed to find account with address %s", kp.Address(s.Env())) - } - - log.Debugf("checking locked wallet, address=%s balance=%v", kp.Address(s.Env()), balance.Balance) + log.Debugf("checking locked wallet, address=%s balance=%d blocks-to-unlock=%d", + lockedAddr, balance.Balance, balance.BlocksToUnlock) if balance.Balance < uint64(s.receivedAmountInPiconero()) { return nil, fmt.Errorf("locked XMR amount is less than expected: got %v, expected %v", balance.Balance, float64(s.receivedAmountInPiconero())) } - // also check that the balance isn't unlocked only after an unreasonable amount of blocks - // somewhat arbitrarily chosen as 10x the default unlock time - // maybe make this configurable? - if balance.BlocksToUnlock > 100 { - return nil, fmt.Errorf("locked XMR unlocks too far into the future: got %d blocks", balance.BlocksToUnlock) + // Monero received from a transfer is locked for a minimum of 10 confirmations before + // it can be spent again. The maker is required to wait for 10 confirmations before + // notifying us that the XMR is locked and should not be adding additional wait + // requirements. We give one block of leniency, in case the taker's node is not fully + // synced. Our goal is to prevent double spends, issues due to block reorgs, and + // prevent the maker from locking our funds until close to the heat death of the + // universe (https://github.com/monero-project/research-lab/issues/78). + if balance.BlocksToUnlock > 1 { + return nil, fmt.Errorf("received XMR funds are not unlocked as required (blocks-to-unlock=%d)", + balance.BlocksToUnlock) } if err := s.CloseWallet(); err != nil { diff --git a/protocol/xmrtaker/recovery.go b/protocol/xmrtaker/recovery.go index 9efb58ad..9e73ab49 100644 --- a/protocol/xmrtaker/recovery.go +++ b/protocol/xmrtaker/recovery.go @@ -47,18 +47,19 @@ func NewRecoveryState(b backend.Backend, dataDir string, secret *mcrypto.Private ctx, cancel := context.WithCancel(b.Ctx()) s := &swapState{ - ctx: ctx, - cancel: cancel, - Backend: b, - sender: sender, - privkeys: kp, - pubkeys: pubkp, - dleqProof: dleq.NewProofWithSecret(sc), - contractSwapID: contractSwapID, - contractSwap: contractSwap, - infoFile: pcommon.GetSwapRecoveryFilepath(dataDir), - claimedCh: make(chan struct{}), - info: pswap.NewEmptyInfo(), + ctx: ctx, + cancel: cancel, + Backend: b, + sender: sender, + privkeys: kp, + pubkeys: pubkp, + dleqProof: dleq.NewProofWithSecret(sc), + walletScanHeight: 0, // TODO: Can we optimise this? + contractSwapID: contractSwapID, + contractSwap: contractSwap, + infoFile: pcommon.GetSwapRecoveryFilepath(dataDir), + claimedCh: make(chan struct{}), + info: pswap.NewEmptyInfo(), } rs := &recoveryState{ diff --git a/protocol/xmrtaker/swap_state.go b/protocol/xmrtaker/swap_state.go index bb4d0adc..53491b26 100644 --- a/protocol/xmrtaker/swap_state.go +++ b/protocol/xmrtaker/swap_state.go @@ -58,6 +58,9 @@ type swapState struct { xmrmakerSecp256k1PublicKey *secp256k1.PublicKey xmrmakerAddress ethcommon.Address + // block height at start of swap used for fast wallet creation + walletScanHeight uint64 + // ETH asset being swapped ethAsset types.EthAsset @@ -119,6 +122,15 @@ func newSwapState(b backend.Backend, offerID types.Hash, infofile string, transf } } + walletScanHeight, err := b.GetChainHeight() + if err != nil { + return nil, err + } + // reduce the scan height a little in case there is a block reorg + if walletScanHeight >= monero.MinSpendConfirmations { + walletScanHeight -= monero.MinSpendConfirmations + } + ctx, cancel := context.WithCancel(b.Ctx()) s := &swapState{ ctx: ctx, @@ -127,6 +139,7 @@ func newSwapState(b backend.Backend, offerID types.Hash, infofile string, transf sender: sender, infoFile: infofile, transferBack: transferBack, + walletScanHeight: walletScanHeight, nextExpectedMessage: &net.SendKeysMessage{}, xmrLockedCh: make(chan struct{}), claimedCh: make(chan struct{}), @@ -595,7 +608,7 @@ func (s *swapState) claimMonero(skB *mcrypto.PrivateSpendKey) (mcrypto.Address, s.LockClient() defer s.UnlockClient() - addr, err := monero.CreateWallet("xmrtaker-swap-wallet", s.Env(), s.Backend, kpAB) + addr, err := monero.CreateWallet("xmrtaker-swap-wallet", s.Env(), s.Backend, kpAB, s.walletScanHeight) if err != nil { return "", err } @@ -625,21 +638,11 @@ func (s *swapState) claimMonero(skB *mcrypto.PrivateSpendKey) (mcrypto.Address, return "", fmt.Errorf("failed to wait for balance to unlock: %w", err) } - res, err := s.SweepAll(depositAddr, 0) + _, err = s.SweepAll(depositAddr, 0) if err != nil { return "", fmt.Errorf("failed to send funds to original account: %w", err) } - if len(res.AmountList) == 0 { - return "", fmt.Errorf("sweep all did not return any amounts") - } - - amount := res.AmountList[0] - log.Infof("transferred %v XMR to %s", - common.MoneroAmount(amount).AsMonero(), - depositAddr, - ) - close(s.claimedCh) return addr, nil } @@ -659,7 +662,7 @@ func (s *swapState) waitUntilBalanceUnlocks() error { if balance.Balance == balance.UnlockedBalance { return nil } - if _, err = monero.WaitForBlocks(s, int(balance.BlocksToUnlock)); err != nil { + if _, err = monero.WaitForBlocks(s.ctx, s, int(balance.BlocksToUnlock)); err != nil { log.Warnf("Waiting for %d monero blocks failed: %s", balance.BlocksToUnlock, err) } } diff --git a/protocol/xmrtaker/swap_state_test.go b/protocol/xmrtaker/swap_state_test.go index aa237a6d..3e8f8720 100644 --- a/protocol/xmrtaker/swap_state_test.go +++ b/protocol/xmrtaker/swap_state_test.go @@ -321,9 +321,20 @@ func TestSwapState_NotifyClaimed(t *testing.T) { t.Logf("transferred %d pico XMR (fees %d) to account %s", tResp.Amount, tResp.Fee, xmrAddr) require.Equal(t, uint64(amt), tResp.Amount) + transfer, err := backend.WaitForTransReceipt(&monero.WaitForReceiptRequest{ + Ctx: s.ctx, + TxID: tResp.TxHash, + DestAddr: xmrAddr, + NumConfirmations: monero.MinSpendConfirmations, + AccountIdx: 0, + }) + require.NoError(t, err) + t.Logf("Transfer mined at block=%d with %d confirmations", transfer.Height, transfer.Confirmations) + // send notification that monero was locked lmsg := &message.NotifyXMRLock{ Address: string(xmrAddr), + TxID: transfer.TxID, } resp, done, err = s.HandleProtocolMessage(lmsg) diff --git a/recover/recovery.go b/recover/recovery.go index 65aaa87c..e7aa35de 100644 --- a/recover/recovery.go +++ b/recover/recovery.go @@ -64,7 +64,7 @@ func (r *recoverer) WalletFromSecrets(xmrtakerSecret, xmrmakerSecret string) (mc return "", err } - return monero.CreateWallet("recovered-wallet", r.env, r.xmrClient, kp) + return monero.CreateWallet("recovered-wallet", r.env, r.xmrClient, kp, 0) } // WalletFromSharedSecret generates a monero wallet from the given shared secret. @@ -85,7 +85,7 @@ func (r *recoverer) WalletFromSharedSecret(pk *mcrypto.PrivateKeyInfo) (mcrypto. } kp := mcrypto.NewPrivateKeyPair(sk, vk) - return monero.CreateWallet("recovered-wallet", r.env, r.xmrClient, kp) + return monero.CreateWallet("recovered-wallet", r.env, r.xmrClient, kp, 0) } // RecoverFromXMRMakerSecretAndContract recovers funds by either claiming ether or reclaiming locked monero. diff --git a/recover/recovery_test.go b/recover/recovery_test.go index 5a156296..325ec8bc 100644 --- a/recover/recovery_test.go +++ b/recover/recovery_test.go @@ -21,7 +21,7 @@ import ( "github.com/athanorlabs/atomic-swap/tests" ) -var defaultTimeout int64 = 8 // 8 seconds +var defaultTimeout int64 = 9 // timeout in seconds func newRecoverer(t *testing.T) *recoverer { r, err := NewRecoverer(common.Development, monero.CreateWalletClient(t), common.DefaultEthEndpoint) diff --git a/rpcclient/personal.go b/rpcclient/personal.go index 9f57f67c..8217e20f 100644 --- a/rpcclient/personal.go +++ b/rpcclient/personal.go @@ -9,13 +9,13 @@ import ( ) // SetSwapTimeout calls personal_setSwapTimeout. -func (c *Client) SetSwapTimeout(duration uint64) error { +func (c *Client) SetSwapTimeout(timeoutSeconds uint64) error { const ( method = "personal_setSwapTimeout" ) req := &rpc.SetSwapTimeoutRequest{ - Timeout: duration, + Timeout: timeoutSeconds, } params, err := json.Marshal(req) diff --git a/scripts/testlib.sh b/scripts/testlib.sh index 0b669a57..e420bd56 100755 --- a/scripts/testlib.sh +++ b/scripts/testlib.sh @@ -121,6 +121,10 @@ start-monerod-regtest() { --rpc-bind-ip=127.0.0.1 \ --rpc-bind-port=18081 sleep 5 + # Make sure the blockchain has some initial decoy outputs. Arbitrarily sending the + # rewards to the Mastering Monero address. + local rewardsAddr=4BKjy1uVRTPiz4pHyaXXawb82XpzLiowSDd8rEQJGqvN6AD6kWosLQ6VJXW9sghopxXgQSh1RTd54JdvvCRsXiF41xvfeW5 + mine-monero "${rewardsAddr}" >/dev/null } stop-monerod-regtest() { diff --git a/tests/integration_test.go b/tests/integration_test.go index e1db8cf4..2cd71143 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -2,7 +2,6 @@ package tests import ( "context" - "errors" "fmt" "os" "strings" @@ -38,6 +37,8 @@ const ( defaultDiscoverTimeout = 2 // 2 seconds + defaultSwapTimeout = 90 // number of seconds that we reset the taker's swap timeout to between tests + xmrmakerProvideAmount = float64(1.0) exchangeRate = float64(0.05) ) @@ -59,6 +60,13 @@ func (s *IntegrationTestSuite) SetupTest() { // We need slightly more than xmrmakerProvideAmount for transaction fees mineMinXMRMakerBalance(s.T(), common.MoneroToPiconero(xmrmakerProvideAmount*2)) } + s.setSwapTimeout(defaultSwapTimeout) // reset between tests, so every test starts in a known state +} + +func (s *IntegrationTestSuite) setSwapTimeout(timeoutSeconds uint64) { + ac := rpcclient.NewClient(defaultXMRTakerSwapdEndpoint) + err := ac.SetSwapTimeout(timeoutSeconds) + require.NoError(s.T(), err) } // mineMinXMRMakerBalance is similar to monero.MineMinXMRBalance(...), but this version @@ -83,6 +91,15 @@ func mineMinXMRMakerBalance(t *testing.T, minBalance common.MoneroAmount) { } } +func (s *IntegrationTestSuite) newSwapdWSClient(ctx context.Context, endpoint string) wsclient.WsClient { + wsc, err := wsclient.NewWsClient(ctx, endpoint) + require.NoError(s.T(), err) + s.T().Cleanup(func() { + wsc.Close() + }) + return wsc +} + func (s *IntegrationTestSuite) TestXMRTaker_Discover() { bc := rpcclient.NewClient(defaultXMRMakerSwapdEndpoint) _, err := bc.MakeOffer(xmrmakerProvideAmount, xmrmakerProvideAmount, exchangeRate, types.EthAssetETH, "", 0) @@ -155,12 +172,10 @@ func (s *IntegrationTestSuite) testSuccessOneSwap( ) { const testTimeout = time.Second * 75 - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() - bwsc, err := wsclient.NewWsClient(ctx, defaultXMRMakerSwapdWSEndpoint) - require.NoError(s.T(), err) - + bwsc := s.newSwapdWSClient(ctx, defaultXMRMakerSwapdWSEndpoint) offerID, statusCh, err := bwsc.MakeOfferAndSubscribe(0.1, xmrmakerProvideAmount, types.ExchangeRate(exchangeRate), asset, relayerEndpoint, relayerCommission) require.NoError(s.T(), err) @@ -187,21 +202,19 @@ func (s *IntegrationTestSuite) testSuccessOneSwap( if status.IsOngoing() { continue } - if status != types.CompletedSuccess { errCh <- fmt.Errorf("swap did not complete successfully: got %s", status) } return - case <-time.After(testTimeout): - errCh <- errors.New("make offer subscription timed out") + case <-ctx.Done(): + errCh <- fmt.Errorf("make offer context canceled: %w", ctx.Err()) return } } }() ac := rpcclient.NewClient(defaultXMRTakerSwapdEndpoint) - awsc, err := wsclient.NewWsClient(ctx, defaultXMRTakerSwapdWSEndpoint) - require.NoError(s.T(), err) + awsc := s.newSwapdWSClient(ctx, defaultXMRTakerSwapdWSEndpoint) // TODO: implement discovery over websockets (#97) providers, err := ac.Discover(types.ProvidesXMR, defaultDiscoverTimeout) @@ -249,12 +262,10 @@ func (s *IntegrationTestSuite) testRefundXMRTakerCancels(asset types.EthAsset) { swapTimeout = 30 ) - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() - bwsc, err := wsclient.NewWsClient(ctx, defaultXMRMakerSwapdWSEndpoint) - require.NoError(s.T(), err) - + bwsc := s.newSwapdWSClient(ctx, defaultXMRMakerSwapdWSEndpoint) offerID, statusCh, err := bwsc.MakeOfferAndSubscribe(0.1, xmrmakerProvideAmount, types.ExchangeRate(exchangeRate), asset, "", 0) require.NoError(s.T(), err) @@ -291,19 +302,17 @@ func (s *IntegrationTestSuite) testRefundXMRTakerCancels(asset types.EthAsset) { errCh <- fmt.Errorf("swap did not succeed or refund for XMRMaker: status=%s", status) } return - case <-time.After(testTimeout): - errCh <- errors.New("make offer subscription timed out") + case <-ctx.Done(): + errCh <- fmt.Errorf("make offer context canceled: %w", ctx.Err()) return } } }() ac := rpcclient.NewClient(defaultXMRTakerSwapdEndpoint) - awsc, err := wsclient.NewWsClient(ctx, defaultXMRTakerSwapdWSEndpoint) - require.NoError(s.T(), err) + awsc := s.newSwapdWSClient(ctx, defaultXMRTakerSwapdWSEndpoint) - err = ac.SetSwapTimeout(swapTimeout) - require.NoError(s.T(), err) + s.setSwapTimeout(swapTimeout) providers, err := ac.Discover(types.ProvidesXMR, defaultDiscoverTimeout) require.NoError(s.T(), err) @@ -385,14 +394,13 @@ func (s *IntegrationTestSuite) testRefundXMRMakerCancels( //nolint:unused ) { const testTimeout = time.Second * 60 - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() bc := rpcclient.NewClient(defaultXMRMakerSwapdEndpoint) - bwsc, err := wsclient.NewWsClient(ctx, defaultXMRMakerSwapdWSEndpoint) - require.NoError(s.T(), err) + bwsc := s.newSwapdWSClient(ctx, defaultXMRMakerSwapdWSEndpoint) defer func() { - err = bc.ClearOffers(nil) + err := bc.ClearOffers(nil) require.NoError(s.T(), err) }() @@ -433,18 +441,17 @@ func (s *IntegrationTestSuite) testRefundXMRMakerCancels( //nolint:unused s.T().Log("> XMRMaker refunded successfully") return - case <-time.After(testTimeout): - errCh <- errors.New("make offer subscription timed out") + case <-ctx.Done(): + errCh <- fmt.Errorf("make offer context canceled: %w", ctx.Err()) + return } } }() ac := rpcclient.NewClient(defaultXMRTakerSwapdEndpoint) - awsc, err := wsclient.NewWsClient(ctx, defaultXMRTakerSwapdWSEndpoint) - require.NoError(s.T(), err) + awsc := s.newSwapdWSClient(ctx, defaultXMRTakerSwapdWSEndpoint) - err = ac.SetSwapTimeout(swapTimeout) - require.NoError(s.T(), err) + s.setSwapTimeout(swapTimeout) providers, err := ac.Discover(types.ProvidesXMR, defaultDiscoverTimeout) require.NoError(s.T(), err) @@ -498,11 +505,10 @@ func (s *IntegrationTestSuite) TestAbort_XMRTakerCancels() { func (s *IntegrationTestSuite) testAbortXMRTakerCancels(asset types.EthAsset) { const testTimeout = time.Second * 60 - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() - bwsc, err := wsclient.NewWsClient(ctx, defaultXMRMakerSwapdWSEndpoint) - require.NoError(s.T(), err) + bwsc := s.newSwapdWSClient(ctx, defaultXMRMakerSwapdWSEndpoint) offerID, statusCh, err := bwsc.MakeOfferAndSubscribe(0.1, xmrmakerProvideAmount, types.ExchangeRate(exchangeRate), asset, "", 0) @@ -537,15 +543,15 @@ func (s *IntegrationTestSuite) testAbortXMRTakerCancels(asset types.EthAsset) { } return - case <-time.After(testTimeout): - errCh <- errors.New("make offer subscription timed out") + case <-ctx.Done(): + errCh <- fmt.Errorf("make offer context canceled: %w", ctx.Err()) + return } } }() ac := rpcclient.NewClient(defaultXMRTakerSwapdEndpoint) - awsc, err := wsclient.NewWsClient(ctx, defaultXMRTakerSwapdWSEndpoint) - require.NoError(s.T(), err) + awsc := s.newSwapdWSClient(ctx, defaultXMRTakerSwapdWSEndpoint) providers, err := ac.Discover(types.ProvidesXMR, defaultDiscoverTimeout) require.NoError(s.T(), err) @@ -602,12 +608,11 @@ func (s *IntegrationTestSuite) TestAbort_XMRMakerCancels() { func (s *IntegrationTestSuite) testAbortXMRMakerCancels(asset types.EthAsset) { const testTimeout = time.Second * 60 - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() bcli := rpcclient.NewClient(defaultXMRMakerSwapdEndpoint) - bwsc, err := wsclient.NewWsClient(ctx, defaultXMRMakerSwapdWSEndpoint) - require.NoError(s.T(), err) + bwsc := s.newSwapdWSClient(ctx, defaultXMRMakerSwapdWSEndpoint) offerID, statusCh, err := bwsc.MakeOfferAndSubscribe(0.1, xmrmakerProvideAmount, types.ExchangeRate(exchangeRate), asset, "", 0) @@ -636,30 +641,27 @@ func (s *IntegrationTestSuite) testAbortXMRMakerCancels(asset types.EthAsset) { if status != types.KeysExchanged { continue } - s.T().Log("> XMRMaker cancelled swap!") exitStatus, err := bcli.Cancel(offerID) //nolint:govet if err != nil { errCh <- err return } - if exitStatus != types.CompletedAbort { errCh <- fmt.Errorf("did not abort successfully: exit status was %s", exitStatus) return } - s.T().Log("> XMRMaker exited successfully") return - case <-time.After(testTimeout): - errCh <- errors.New("make offer subscription timed out") + case <-ctx.Done(): + errCh <- fmt.Errorf("make offer context canceled: %w", ctx.Err()) + return } } }() c := rpcclient.NewClient(defaultXMRTakerSwapdEndpoint) - wsc, err := wsclient.NewWsClient(ctx, defaultXMRTakerSwapdWSEndpoint) - require.NoError(s.T(), err) + wsc := s.newSwapdWSClient(ctx, defaultXMRTakerSwapdWSEndpoint) providers, err := c.Discover(types.ProvidesXMR, defaultDiscoverTimeout) require.NoError(s.T(), err) @@ -709,7 +711,7 @@ func (s *IntegrationTestSuite) TestError_ShouldOnlyTakeOfferOnce() { func (s *IntegrationTestSuite) testErrorShouldOnlyTakeOfferOnce(asset types.EthAsset) { const testTimeout = time.Second * 60 - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() bc := rpcclient.NewClient(defaultXMRMakerSwapdEndpoint) @@ -735,8 +737,7 @@ func (s *IntegrationTestSuite) testErrorShouldOnlyTakeOfferOnce(asset types.EthA go func() { defer wg.Done() - wsc, err := wsclient.NewWsClient(ctx, defaultXMRTakerSwapdWSEndpoint) - require.NoError(s.T(), err) + wsc := s.newSwapdWSClient(ctx, defaultXMRTakerSwapdWSEndpoint) takerStatusCh, err := wsc.TakeOfferAndSubscribe(providers[0][0], offerID, 0.05) if err != nil { @@ -763,8 +764,7 @@ func (s *IntegrationTestSuite) testErrorShouldOnlyTakeOfferOnce(asset types.EthA go func() { defer wg.Done() - wsc, err := wsclient.NewWsClient(ctx, defaultCharlieSwapdWSEndpoint) - require.NoError(s.T(), err) + wsc := s.newSwapdWSClient(ctx, defaultCharlieSwapdWSEndpoint) takerStatusCh, err := wsc.TakeOfferAndSubscribe(providers[0][0], offerID, 0.05) if err != nil { @@ -794,7 +794,7 @@ func (s *IntegrationTestSuite) testErrorShouldOnlyTakeOfferOnce(asset types.EthA case err := <-errCh: require.NotNil(s.T(), err) s.T().Log("got expected error:", err) - case <-time.After(testTimeout): + case <-ctx.Done(): s.T().Fatalf("did not get error from XMRTaker or Charlie") } @@ -810,34 +810,37 @@ func (s *IntegrationTestSuite) TestSuccess_ConcurrentSwaps() { } func (s *IntegrationTestSuite) testSuccessConcurrentSwaps(asset types.EthAsset) { - const testTimeout = time.Minute * 6 const numConcurrentSwaps = 10 + const swapTimeout = 30 * numConcurrentSwaps + const testTimeout = (swapTimeout * 2 * time.Second) + (1 * time.Minute) - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) defer cancel() + s.setSwapTimeout(swapTimeout) + type makerTest struct { offerID string statusCh <-chan types.Status errCh chan error + index int } + // Create the XMRMaker offers synchronously makerTests := make([]*makerTest, numConcurrentSwaps) - for i := 0; i < numConcurrentSwaps; i++ { - bwsc, err := wsclient.NewWsClient(ctx, defaultXMRMakerSwapdWSEndpoint) - require.NoError(s.T(), err) - + bwsc := s.newSwapdWSClient(ctx, defaultXMRMakerSwapdWSEndpoint) offerID, statusCh, err := bwsc.MakeOfferAndSubscribe(0.1, xmrmakerProvideAmount, types.ExchangeRate(exchangeRate), asset, "", 0) require.NoError(s.T(), err) - s.T().Log("maker made offer ", offerID) + s.T().Logf("XMRMaker[%d] made offer %s", i, offerID) makerTests[i] = &makerTest{ offerID: offerID, statusCh: statusCh, - errCh: make(chan error, 2), + errCh: make(chan error, numConcurrentSwaps), + index: i, } } @@ -852,44 +855,44 @@ func (s *IntegrationTestSuite) testSuccessConcurrentSwaps(asset types.EthAsset) var wg sync.WaitGroup wg.Add(2 * numConcurrentSwaps) - for _, tc := range makerTests { - go func(tc *makerTest) { + // Track each XMRMaker's status asynchronously + for _, mkrTest := range makerTests { + go func(mt *makerTest) { defer wg.Done() for { select { - case status := <-tc.statusCh: - s.T().Log("> XMRMaker got status:", status) + case status := <-mt.statusCh: + s.T().Logf("> XMRMaker[%d] got status: %s", mt.index, status) if status.IsOngoing() { continue } - if status != types.CompletedSuccess { - tc.errCh <- fmt.Errorf("swap did not complete successfully: got %s", status) + mt.errCh <- fmt.Errorf("XMRMaker[%d] swap did not succeed: %s", mt.index, status) } - return - case <-time.After(testTimeout): - tc.errCh <- errors.New("make offer subscription timed out") + case <-ctx.Done(): + mt.errCh <- fmt.Errorf("XMRMaker[%d] context canceled: %w", mt.index, ctx.Err()) + return } } - }(tc) + }(mkrTest) } type takerTest struct { statusCh <-chan types.Status errCh chan error + index int } + // Create the XMRTakers synchronously takerTests := make([]*takerTest, numConcurrentSwaps) - for i := 0; i < numConcurrentSwaps; i++ { ac := rpcclient.NewClient(defaultXMRTakerSwapdEndpoint) - awsc, err := wsclient.NewWsClient(ctx, defaultXMRTakerSwapdWSEndpoint) //nolint:govet - require.NoError(s.T(), err) + awsc := s.newSwapdWSClient(ctx, defaultXMRTakerSwapdWSEndpoint) // TODO: implement discovery over websockets (#97) - providers, err := ac.Discover(types.ProvidesXMR, defaultDiscoverTimeout) + providers, err := ac.Discover(types.ProvidesXMR, defaultDiscoverTimeout) //nolint:govet require.NoError(s.T(), err) require.Equal(s.T(), 1, len(providers)) require.GreaterOrEqual(s.T(), len(providers[0]), 2) @@ -898,33 +901,41 @@ func (s *IntegrationTestSuite) testSuccessConcurrentSwaps(asset types.EthAsset) takerStatusCh, err := awsc.TakeOfferAndSubscribe(providers[0][0], offerID, 0.05) require.NoError(s.T(), err) - s.T().Log("taker took offer ", offerID) + s.T().Logf("XMRTaker[%d] took offer %s", i, offerID) takerTests[i] = &takerTest{ statusCh: takerStatusCh, - errCh: make(chan error, 2), + errCh: make(chan error, numConcurrentSwaps), + index: i, } } - for _, tc := range takerTests { - go func(tc *takerTest) { + // Track each XMRTaker's status asynchronously + for _, tkrTest := range takerTests { + tkrTest := tkrTest + go func(tt *takerTest) { defer wg.Done() - for status := range tc.statusCh { - s.T().Log("> XMRTaker got status:", status) - if status.IsOngoing() { - continue + for { + select { + case status := <-tt.statusCh: + s.T().Logf("> XMRTaker[%d] got status: %s", tt.index, status) + if status.IsOngoing() { + continue + } + if status != types.CompletedSuccess { + tt.errCh <- fmt.Errorf("XMRTaker[%d] did not succeed: %s", tt.index, status) + } + return + case <-ctx.Done(): + tkrTest.errCh <- fmt.Errorf("XMRTaker[%d] context ended: %w", tt.index, ctx.Err()) + return } - - if status != types.CompletedSuccess { - tc.errCh <- fmt.Errorf("swap did not complete successfully: got %s", status) - } - - return } - }(tc) + }(tkrTest) } wg.Wait() + s.T().Logf("All %d XMR makers and takers completed", numConcurrentSwaps) for _, tc := range makerTests { select { diff --git a/tests/monero_wallet_rpc.go b/tests/monero_wallet_rpc.go deleted file mode 100644 index 127552c9..00000000 --- a/tests/monero_wallet_rpc.go +++ /dev/null @@ -1,93 +0,0 @@ -package tests - -import ( - "bufio" - "fmt" - "net" - "os/exec" - "path" - "runtime" - "strings" - "testing" - "time" - - "github.com/stretchr/testify/require" -) - -// CreateWalletRPCService starts a monero-wallet-rpc listening on a random port for tests. The json_rpc -// URL of the started service is returned. -func CreateWalletRPCService(t *testing.T) string { - port := getFreePort(t) - walletRPCBin := getMoneroWalletRPCBin(t) - walletRPCBinArgs := getWalletRPCFlags(t, port) - cmd := exec.Command(walletRPCBin, walletRPCBinArgs...) - outPipe, err := cmd.StdoutPipe() - require.NoError(t, err) - - err = cmd.Start() - require.NoError(t, err) - t.Cleanup(func() { - _ = outPipe.Close() - _ = cmd.Process.Kill() - _ = cmd.Wait() - }) - scanner := bufio.NewScanner(outPipe) - started := false - for scanner.Scan() { - line := scanner.Text() - //t.Log(line) - if strings.HasSuffix(line, "Starting wallet RPC server") { - started = true - break - } - time.Sleep(200 * time.Millisecond) // additional start time - } - if !started { - t.Fatal("failed to start monero-wallet-rpc") - } - - // drain any additional output - go func() { - for scanner.Scan() { - //t.Log(scanner.Text()) - } - }() - - require.NoError(t, err) - return fmt.Sprintf("http://127.0.0.1:%d/json_rpc", port) -} - -// getMoneroWalletRPCBin returns the monero-wallet-rpc binary assuming it was -// installed at the top of the repo in a directory named "monero-bin". -func getMoneroWalletRPCBin(t *testing.T) string { - _, filename, _, ok := runtime.Caller(0) // this test file path - require.True(t, ok) - packageDir := path.Dir(filename) - repoBaseDir := path.Dir(packageDir) - return path.Join(repoBaseDir, "monero-bin", "monero-wallet-rpc") -} - -// getWalletRPCFlags returns the flags used when launching monero-wallet-rpc in a temporary -// test folder. -func getWalletRPCFlags(t *testing.T, port int) []string { - walletDir := t.TempDir() - return []string{ - "--rpc-bind-ip=127.0.0.1", - fmt.Sprintf("--rpc-bind-port=%d", port), - "--disable-rpc-login", - fmt.Sprintf("--log-file=%s", path.Join(walletDir, "monero-wallet-rpc.log")), - fmt.Sprintf("--wallet-dir=%s", t.TempDir()), - "--allow-mismatched-daemon-version", - } -} - -// getFreePort returns an OS allocated and immediately freed port. There is nothing preventing -// something else on the system from using the port before the caller has a chance, but OS -// allocated ports are randomised to minimise this risk. -func getFreePort(t *testing.T) int { - ln, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - port := ln.Addr().(*net.TCPAddr).Port - require.NoError(t, ln.Close()) - return port -}