Idempotent bids #13
Idempotent bids #13
Conversation
| @@ -79,6 +80,9 @@ class Wallet extends EventEmitter { | |||
|
|
|||
| this.txdb = new TXDB(this.wdb); | |||
|
|
|||
| const capacity = consensus.MAX_BLOCK_OPENS * 10; | |||
| this.sendBidResults = new LRU(capacity); | |||
There was a problem hiding this comment.
Initially thought we were just storing the keys but we're also storing the responses. What was your reasoning for this cache size? A number that makes sense to me is 20 blocks worth of bids (current conf time), which would be auctions per block * bids per auction * 20. With the max auctions per block, this would be:
const bidsCacheSizeInBlocks = 20;
const bidsPerAuction = 3; // the average right now is 3.1 ish
const capacity = consensus.MAX_BLOCK_OPENS * bidsPerAuction * bidsCacheSizeInBlocks;What do you think?
There was a problem hiding this comment.
I was thinking that we just needed to store the last couple of blocks worth of bids, but I like your reasoning. I think it's fine to provision a little extra. The JSON serialization of a bid is about 2 KB, so 60 * 300 * 2KB = about 36 MB, which is fine.
The main goal here is to just avoid using unbounded memory if the fullnode runs for weeks without ever restarting.
After thinking about it more, we need to provision for peak, which is when a big week's worth of auctions close within a few blocks of each other. I'll increase it to opens * 20 *3.
lib/wallet/wallet.js
Outdated
| const idempotencyKey = options ? options.idempotencyKey : null; | ||
| if (idempotencyKey) { | ||
| if (this.sendBidResults.has(idempotencyKey)) { | ||
| return this.sendBidResults.get(idempotencyKey);; |
There was a problem hiding this comment.
semicolon; does your editor running this repo's eslint?
lib/wallet/wallet.js
Outdated
| if (idempotencyKey) { | ||
| this.sendBidResults.set(idempotencyKey, result); | ||
| } | ||
| return result |
Add in-memory idempotency cache to prevent double-sending bids