Skip to content

Encapsulate relay cache + new tests#496

Open
dagurval wants to merge 1 commit into
bitcoinxt:masterfrom
dagurval:relaycache
Open

Encapsulate relay cache + new tests#496
dagurval wants to merge 1 commit into
bitcoinxt:masterfrom
dagurval:relaycache

Conversation

@dagurval

Copy link
Copy Markdown
Member

No description provided.

@sickpig

sickpig commented Oct 19, 2018

Copy link
Copy Markdown
Collaborator

Ok this was quick! Keep up the good work @Dagur! 😄

@dgenr8 dgenr8 requested a review from sickpig October 22, 2018 14:38
Comment thread src/main.cpp
Comment thread src/relaycache.cpp
return;
}
int64_t time = GetTime();
while (expiration.front().first <= time) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

should be expiration.empty() && expiration.front() ... here

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

!expiration.empty() but agreed, and suggest remove lines 11-13.

Comment thread src/net.h
Comment thread src/relaycache.cpp
return;
}
int64_t time = GetTime();
while (expiration.front().first <= time) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

!expiration.empty() but agreed, and suggest remove lines 11-13.

Comment thread src/relaycache.cpp
return ::GetTime();
}

void RelayCache::RemoveExpiration(const uint256& txid) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

assert(bool(cs)); ?

Comment thread src/relaycache.cpp
}

void RelayCache::RemoveExpiration(const uint256& txid) {
for (auto e = begin(expiration); e != end(expiration); ++e) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Inefficient but I guess we should rarely get here.


// Expire tx1
cache.time = 100 + RELAY_CACHE_TIMEOUT;
BOOST_CHECK(cache.FindTx(tx1.GetHash()).IsNull());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since ExpireOld() is not called, expiration is not being tested. IsNull() passes because setting the version is not sufficient to make it not pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants