simplify TradeManager.respond_to_offer() return (#13927)

* simplify TradeManager.respond_to_offer() return

* require the exception

* correct to match=

* another failure case
This commit is contained in:
Kyle Altendorf 2022-11-21 21:29:35 -05:00 committed by GitHub
parent 5cbc415589
commit caa7e42845
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 35 additions and 101 deletions

View file

@ -1551,12 +1551,9 @@ class WalletRpcApi:
peer: Optional[WSChiaConnection] = self.service.get_full_node_peer()
if peer is None:
raise ValueError("No peer connected")
result = await self.service.wallet_state_manager.trade_manager.respond_to_offer(
trade_record, tx_records = await self.service.wallet_state_manager.trade_manager.respond_to_offer(
offer, peer, fee=fee, min_coin_amount=min_coin_amount, max_coin_amount=max_coin_amount, solver=solver
)
if not result[0]:
raise ValueError(result[3])
success, trade_record, tx_records, error = result
return {"trade_record": trade_record.to_json_dict_convenience()}
async def get_offer(self, request: Dict) -> EndpointResult:

View file

@ -703,9 +703,7 @@ class TradeManager:
fee: uint64 = uint64(0),
min_coin_amount: Optional[uint64] = None,
max_coin_amount: Optional[uint64] = None,
) -> Union[
Tuple[Literal[True], TradeRecord, List[TransactionRecord], None], Tuple[Literal[False], None, None, str]
]:
) -> Tuple[TradeRecord, List[TransactionRecord]]:
if solver is None:
solver = Solver({})
take_offer_dict: Dict[Union[bytes32, int], int] = {}
@ -719,7 +717,7 @@ class TradeManager:
# ATTENTION: new wallets
wallet = await self.wallet_state_manager.get_wallet_for_asset_id(asset_id.hex())
if wallet is None and amount < 0:
return False, None, None, f"Do not have a wallet for asset ID: {asset_id} to fulfill offer"
raise ValueError(f"Do not have a wallet for asset ID: {asset_id} to fulfill offer")
elif wallet is None or wallet.type() in [WalletType.NFT, WalletType.DATA_LAYER]:
key = asset_id
else:
@ -729,7 +727,7 @@ class TradeManager:
# First we validate that all of the coins in this offer exist
valid: bool = await self.check_offer_validity(offer, peer)
if not valid:
return False, None, None, "This offer is no longer valid"
raise ValueError("This offer is no longer valid")
result = await self._create_offer_for_ids(
take_offer_dict,
offer.driver_dict,
@ -739,7 +737,7 @@ class TradeManager:
max_coin_amount=max_coin_amount,
)
if not result[0] or result[1] is None:
return False, None, None, result[2]
raise ValueError(result[2])
success, take_offer, error = result
@ -790,7 +788,7 @@ class TradeManager:
for tx in tx_records:
await self.wallet_state_manager.add_transaction(tx)
return True, trade_record, [push_tx, *tx_records], None
return trade_record, [push_tx, *tx_records]
async def check_for_special_offer_making(
self,

View file

@ -128,11 +128,9 @@ class TestCATTrades:
peer = wallet_node_taker.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=uint64(1)
)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -178,11 +176,7 @@ class TestCATTrades:
assert success is True
assert trade_make is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer
)
assert error is None
assert success is True
trade_take, tx_records = await trade_manager_taker.respond_to_offer(Offer.from_bytes(trade_make.offer), peer)
assert trade_take is not None
assert tx_records is not None
@ -220,12 +214,8 @@ class TestCATTrades:
assert error is None
assert success is True
assert trade_make is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer
)
trade_take, tx_records = await trade_manager_taker.respond_to_offer(Offer.from_bytes(trade_make.offer), peer)
await time_out_assert(15, full_node.txs_in_mempool, True, tx_records)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -261,12 +251,8 @@ class TestCATTrades:
assert error is None
assert success is True
assert trade_make is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer
)
trade_take, tx_records = await trade_manager_taker.respond_to_offer(Offer.from_bytes(trade_make.offer), peer)
await time_out_assert(15, full_node.txs_in_mempool, True, tx_records)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -302,12 +288,8 @@ class TestCATTrades:
assert error is None
assert success is True
assert trade_make is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer
)
trade_take, tx_records = await trade_manager_taker.respond_to_offer(Offer.from_bytes(trade_make.offer), peer)
await time_out_assert(15, full_node.txs_in_mempool, True, tx_records)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -343,12 +325,8 @@ class TestCATTrades:
assert error is None
assert success is True
assert trade_make is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer
)
trade_take, tx_records = await trade_manager_taker.respond_to_offer(Offer.from_bytes(trade_make.offer), peer)
await time_out_assert(15, full_node.txs_in_mempool, True, tx_records)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -440,12 +418,10 @@ class TestCATTrades:
# Due to current mempool rules, trying to force a take out of the mempool with a cancel will not work.
# Uncomment this when/if it does
# success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
# trade_take, tx_records = await trade_manager_taker.respond_to_offer(
# Offer.from_bytes(trade_make.offer),
# )
# await time_out_assert(15, full_node.txs_in_mempool, True, tx_records)
# assert error is None
# assert success is True
# assert trade_take is not None
# assert tx_records is not None
# await time_out_assert(15, get_trade_and_status, TradeStatus.PENDING_CONFIRM, trade_manager_taker, trade_take)
@ -481,13 +457,8 @@ class TestCATTrades:
peer = wallet_node_taker.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer
)
assert error is not None
assert success is False
assert trade_take is None
assert tx_records is None
with pytest.raises(ValueError, match="This offer is no longer valid"):
await trade_manager_taker.respond_to_offer(Offer.from_bytes(trade_make.offer), peer)
# Now we're going to create the other way around for test coverage sake
success, trade_make, error = await trade_manager_maker.create_offer_for_ids(chia_for_cat)
@ -496,13 +467,11 @@ class TestCATTrades:
assert trade_make is not None
# This take should fail since we have no CATs to fulfill it with
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer
)
assert error is not None
assert success is False
assert trade_take is None
assert tx_records is None
with pytest.raises(
ValueError,
match=f"Do not have a wallet for asset ID: {cat_wallet_maker.get_asset_id()} to fulfill offer",
):
await trade_manager_taker.respond_to_offer(Offer.from_bytes(trade_make.offer), peer)
txs = await trade_manager_maker.cancel_pending_offer_safely(trade_make.trade_id, fee=uint64(0))
await time_out_assert(15, get_trade_and_status, TradeStatus.PENDING_CANCEL, trade_manager_maker, trade_make)

View file

@ -142,7 +142,7 @@ async def test_dl_offers(wallets_prefarm: Any, trusted: bool) -> None:
]
}
success, offer_taker, tx_records, error = await trade_manager_taker.respond_to_offer(
offer_taker, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(offer_maker.offer),
peer,
solver=Solver(
@ -172,8 +172,6 @@ async def test_dl_offers(wallets_prefarm: Any, trusted: bool) -> None:
),
fee=fee,
)
assert error is None
assert success is True
assert offer_taker is not None
assert tx_records is not None
@ -421,7 +419,7 @@ async def test_multiple_dl_offers(wallets_prefarm: Any, trusted: bool) -> None:
assert success is True
assert offer_maker is not None
success, offer_taker, tx_records, error = await trade_manager_taker.respond_to_offer(
offer_taker, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(offer_maker.offer),
peer,
solver=Solver(
@ -464,8 +462,6 @@ async def test_multiple_dl_offers(wallets_prefarm: Any, trusted: bool) -> None:
),
fee=fee,
)
assert error is None
assert success is True
assert offer_taker is not None
assert tx_records is not None

View file

@ -180,14 +180,12 @@ async def test_nft_offer_sell_nft(two_wallet_nodes: Any, trusted: Any) -> None:
peer = wallet_node_taker.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=uint64(taker_fee)
)
await time_out_assert(20, mempool_not_empty, True, full_node_api)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -340,12 +338,10 @@ async def test_nft_offer_request_nft(two_wallet_nodes: Any, trusted: Any) -> Non
peer = wallet_node_taker.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=uint64(taker_fee)
)
await time_out_assert(20, mempool_not_empty, True, full_node_api)
assert error is None
assert success is True
assert trade_take is not None
await full_node_api.process_transaction_records(records=tx_records)
@ -516,12 +512,10 @@ async def test_nft_offer_sell_did_to_did(two_wallet_nodes: Any, trusted: Any) ->
peer = wallet_node_taker.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=uint64(taker_fee)
)
await time_out_assert(20, mempool_not_empty, True, full_node_api)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -712,12 +706,10 @@ async def test_nft_offer_sell_nft_for_cat(two_wallet_nodes: Any, trusted: Any) -
peer = wallet_node_taker.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=uint64(taker_fee)
)
await time_out_assert(20, mempool_not_empty, True, full_node_api)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -912,12 +904,10 @@ async def test_nft_offer_request_nft_for_cat(two_wallet_nodes: Any, trusted: boo
peer = wallet_node_taker.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=uint64(taker_fee)
)
await time_out_assert(20, mempool_not_empty, True, full_node_api)
assert error is None
assert success is True
assert trade_take is not None
assert tx_records is not None
@ -1396,13 +1386,11 @@ async def test_complex_nft_offer(two_wallet_nodes: Any, trusted: Any) -> None:
assert success
assert trade_make is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer),
wallet_node_taker.get_full_node_peer(),
fee=FEE,
)
assert error is None
assert success
assert trade_take is not None
assert tx_records is not None
await full_node_api.process_transaction_records(records=tx_records)
@ -1497,13 +1485,11 @@ async def test_complex_nft_offer(two_wallet_nodes: Any, trusted: Any) -> None:
assert success
assert trade_make is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer),
wallet_node_taker.get_full_node_peer(),
fee=uint64(0),
)
assert error is None
assert success
assert trade_take is not None
assert tx_records is not None
await time_out_assert(20, mempool_not_empty, True, full_node_api)

View file

@ -128,12 +128,10 @@ async def test_nft_offer_with_fee(two_wallet_nodes: Any, trusted: Any) -> None:
peer = wallet_node_1.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=taker_fee
)
assert success
assert error is None
assert trade_take is not None
assert tx_records is not None
@ -172,12 +170,10 @@ async def test_nft_offer_with_fee(two_wallet_nodes: Any, trusted: Any) -> None:
taker_fee = uint64(1)
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=taker_fee
)
assert success
assert error is None
assert trade_take is not None
assert tx_records is not None
@ -428,12 +424,10 @@ async def test_nft_offer_with_metadata_update(two_wallet_nodes: Any, trusted: An
peer = wallet_node_1.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=taker_fee
)
assert success
assert error is None
assert trade_take is not None
assert tx_records is not None
@ -585,12 +579,10 @@ async def test_nft_offer_nft_for_cat(two_wallet_nodes: Any, trusted: Any) -> Non
peer = wallet_node_1.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=taker_fee
)
assert success
assert error is None
assert trade_take is not None
assert tx_records is not None
@ -641,12 +633,10 @@ async def test_nft_offer_nft_for_cat(two_wallet_nodes: Any, trusted: Any) -> Non
taker_fee = uint64(1)
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=taker_fee
)
assert success
assert error is None
assert trade_take is not None
assert tx_records is not None
@ -786,12 +776,10 @@ async def test_nft_offer_nft_for_nft(two_wallet_nodes: Any, trusted: Any) -> Non
peer = wallet_node_1.get_full_node_peer()
assert peer is not None
success, trade_take, tx_records, error = await trade_manager_taker.respond_to_offer(
trade_take, tx_records = await trade_manager_taker.respond_to_offer(
Offer.from_bytes(trade_make.offer), peer, fee=taker_fee
)
assert success
assert error is None
assert trade_take is not None
assert tx_records is not None