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.

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

[BUG] OrderBook update logic typo #4790

Open
sublimator opened this issue Oct 29, 2023 · 3 comments
Open

[BUG] OrderBook update logic typo #4790

sublimator opened this issue Oct 29, 2023 · 3 comments
Assignees

Comments

@sublimator
Copy link
Contributor

void
OrderBookDB::setup(std::shared_ptr<ReadView const> const& ledger)
{
    if (!app_.config().standalone() && app_.getOPs().isNeedNetworkLedger())
    {
        JLOG(j_.warn()) << "Eliding full order book update: no ledger";
        return;
    }

    auto seq = seq_.load();

    if (seq != 0)
    {
        if ((seq > ledger->seq()) && ((ledger->seq() - seq) < 25600))
            return;

        if ((ledger->seq() <= seq) && ((seq - ledger->seq()) < 16))
            return;
    }

This part seems a bit confusing to me:

        if ((seq > ledger->seq()) && ((ledger->seq() - seq) < 25600))

If bigger > smaller && (smaller - bigger) < 25600

The old logic:

        auto seq = ledger->info().seq;

        // Do a full update every 256 ledgers
        if (mSeq != 0)
        {
            if (seq == mSeq)
                return;
            if ((seq > mSeq) && ((seq - mSeq) < 256))
                return;
            if ((seq < mSeq) && ((mSeq - seq) < 16))
                return;
        }
@sublimator
Copy link
Contributor Author

Maybe it was supposed to be:

if ((ledger->seq() > seq) && ((ledger->seq() - seq) < 25600))

I see before the variable seq was used for the passed in ledger' sequence, but now seq is the last sequence (which before was mSeq) so maybe there was some copy/paste issues?

@sublimator sublimator changed the title OrderBook update logic query [BUG] OrderBook update logic query Oct 31, 2023
@sublimator sublimator changed the title [BUG] OrderBook update logic query [BUG] OrderBook update logic typo Oct 31, 2023
@seelabs seelabs self-assigned this Jan 18, 2024
@seelabs
Copy link
Collaborator

seelabs commented Jan 18, 2024

Thanks @sublimator. After a quick look this does look like a bug to me. Let me take a closer look now.

@seelabs
Copy link
Collaborator

seelabs commented Jan 18, 2024

@sublimator There's a PR for this here: #4890

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

No branches or pull requests

2 participants