Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Idempotent bids #13

Merged
merged 4 commits into from
Mar 17, 2020
Merged

Idempotent bids #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99f21dc
add in-memory caching for idempotent bidding
rsolari Mar 17, 2020
e9dc74f
use an LRU cache instead of an unbounded object
rsolari Mar 17, 2020
f558e0d
formatting fixes
rsolari Mar 17, 2020
d3f74ab
increase cache size & show my work
rsolari Mar 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions lib/wallet/rpc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ class RPC extends RPCBase {
this.add('createcancel', this.createCancel);
this.add('createfinalize', this.createFinalize);
this.add('createrevoke', this.createRevoke);
this.add('sendbididempotent', this.sendBidIdempotent);

// Compat
this.add('getauctions', this.getNames);
Expand Down Expand Up @@ -2053,6 +2054,48 @@ class RPC extends RPCBase {
};
}


async sendBidIdempotent(args, help) {
const opts = this._validateBidItempotent(args, help, 'sendbididempotent');
const wallet = this.wallet;
const tx = await wallet.sendBid(opts.name, opts.bid, opts.value, {
account: opts.account,
idempotencyKey: opts.idempotencyKey,
});

return tx.getJSON(this.network);
}

_validateBidItempotent(args, help, method) {
const msg = `${method} "name" bid value idempotencykey ( "account" )`;

if (help || args.length < 4 || args.length > 5)
throw new RPCError(errs.MISC_ERROR, msg);

const valid = new Validator(args);
const name = valid.str(0);
const bid = valid.ufixed(1, EXP);
const value = valid.ufixed(2, EXP);
const idempotencyKey = valid.str(3);
const account = valid.str(4);

if (!name || !rules.verifyName(name))
throw new RPCError(errs.TYPE_ERROR, 'Invalid name.');

if (bid == null || value == null)
throw new RPCError(errs.TYPE_ERROR, 'Invalid values.');

if (bid > value) throw new RPCError(errs.TYPE_ERROR, 'Invalid bid.');

return {
name,
bid,
value,
idempotencyKey,
account
};
}

async sendReveal(args, help) {
const opts = this._validateReveal(args, help, 'sendreveal');
const wallet = this.wallet;
Expand Down
20 changes: 19 additions & 1 deletion lib/wallet/wallet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const base58 = require('bcrypto/lib/encoding/base58');
const bio = require('bufio');
const blake2b = require('bcrypto/lib/blake2b');
const cleanse = require('bcrypto/lib/cleanse');
const LRU = require('blru');
const TXDB = require('./txdb');
const Path = require('./path');
const common = require('./common');
Expand Down Expand Up @@ -79,6 +80,12 @@ class Wallet extends EventEmitter {

this.txdb = new TXDB(this.wdb);

const bidsCacheSizeInBlocks = 20;
const bidsPerAuction = 3; // average in March 2020 is ~3.1
const capacity =
consensus.MAX_BLOCK_OPENS * bidsPerAuction * bidsCacheSizeInBlocks;
this.sendBidResults = new LRU(capacity);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Author

@rsolari rsolari Mar 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


this.maxAncestors = policy.MEMPOOL_MAX_ANCESTORS;

if (options)
Expand Down Expand Up @@ -1795,7 +1802,18 @@ class Wallet extends EventEmitter {
async sendBid(name, value, lockup, options) {
const unlock = await this.fundLock.lock();
try {
return await this._sendBid(name, value, lockup, options);
const idempotencyKey = options ? options.idempotencyKey : null;
if (idempotencyKey) {
if (this.sendBidResults.has(idempotencyKey)) {
return this.sendBidResults.get(idempotencyKey);
}
}

const result = await this._sendBid(name, value, lockup, options);
if (idempotencyKey) {
this.sendBidResults.set(idempotencyKey, result);
}
return result;
} finally {
unlock();
}
Expand Down