Skip to content

dtl: add query param based querying for txs #479

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

Merged
gakonst merged 6 commits into from
Apr 21, 2021
Merged

dtl: add query param based querying for txs #479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0491ff0
dtl: remove stopL2SyncAtHeight config
tynes Apr 20, 2021
5f6cf32
dtl: implement query param based backend
tynes Apr 20, 2021
79df951
dtl: remove dead comment
tynes Apr 20, 2021
f5b8763
dtl: add changeset
tynes Apr 20, 2021
2e92209
dtl: standardize on backend query param
tynes Apr 21, 2021
e86ee09
changeset: update comment
tynes Apr 21, 2021
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
8 changes: 8 additions & 0 deletions .changeset/calm-books-hope.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@eth-optimism/data-transport-layer": patch
---

Allow the DTL to provide data from either L1 or L2, configurable via a query param sent by the client.
The config option `default-backend` can be used to specify the backend to be
used if the query param is not specified. This allows it to be backwards
compatible with how the DTL was previously used.
29 changes: 1 addition & 28 deletions packages/data-transport-layer/src/services/l2-ingestion/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ const optionSettings = {
default: false,
validate: validators.isBoolean,
},
stopL2SyncAtBlock: {
default: Infinity,
validate: validators.isInteger,
},
}

export class L2IngestionService extends BaseService<L2IngestionServiceOptions> {
Expand Down Expand Up @@ -80,30 +76,7 @@ export class L2IngestionService extends BaseService<L2IngestionServiceOptions> {
const highestSyncedL2BlockNumber =
(await this.state.db.getHighestSyncedUnconfirmedBlock()) || 1

// Shut down if we're at the stop block.
if (
this.options.stopL2SyncAtBlock !== undefined &&
this.options.stopL2SyncAtBlock !== null &&
highestSyncedL2BlockNumber >= this.options.stopL2SyncAtBlock
) {
this.logger.info(
"L2 sync is shutting down because we've reached your target block. Goodbye!"
)
return
}

let currentL2Block = await this.state.l2RpcProvider.getBlockNumber()

// Make sure we can't exceed the stop block.
if (
this.options.stopL2SyncAtBlock !== undefined &&
this.options.stopL2SyncAtBlock !== null
) {
currentL2Block = Math.min(
currentL2Block,
this.options.stopL2SyncAtBlock
)
}
const currentL2Block = await this.state.l2RpcProvider.getBlockNumber()

// Make sure we don't exceed the tip.
const targetL2Block = Math.min(
Expand Down
3 changes: 1 addition & 2 deletions packages/data-transport-layer/src/services/main/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@ export interface L1DataTransportServiceOptions {
logsPerPollingInterval: number
pollingInterval: number
port: number
showUnconfirmedTransactions: boolean
syncFromL1?: boolean
syncFromL2?: boolean
transactionsPerPollingInterval: number
legacySequencerCompatibility: boolean
stopL2SyncAtBlock?: number
sentryDsn?: string
defaultBackend: string
}

const optionSettings = {
Expand Down
3 changes: 1 addition & 2 deletions packages/data-transport-layer/src/services/run.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ interface Bcfg {
l2ChainId: config.uint('l2ChainId'),
syncFromL1: config.bool('syncFromL1', true),
syncFromL2: config.bool('syncFromL2', false),
showUnconfirmedTransactions: config.bool('syncFromL2', false),
transactionsPerPollingInterval: config.uint(
'transactionsPerPollingInterval',
1000
Expand All @@ -48,8 +47,8 @@ interface Bcfg {
'legacySequencerCompatibility',
false
),
stopL2SyncAtBlock: config.uint('stopL2SyncAtBlock'),
sentryDsn: config.str('sentryDsn'),
defaultBackend: config.str('defaultBackend', 'l1'),
})

await service.start()
Expand Down
130 changes: 75 additions & 55 deletions packages/data-transport-layer/src/services/server/service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,12 @@ const optionSettings = {
return validators.isUrl(val) || validators.isJsonRpcProvider(val)
},
},
showUnconfirmedTransactions: {
validate: validators.isBoolean,
},
defaultBackend: {
default: 'l1',
validate: (val: string) => {
return val === 'l1' || val === 'l2'
}
}
}

export class L1TransportServer extends BaseService<L1TransportServerOptions> {
Expand All @@ -66,7 +69,6 @@ export class L1TransportServer extends BaseService<L1TransportServerOptions> {
} = {} as any

protected async _init(): Promise<void> {
// TODO: I don't know if this is strictly necessary, but it's probably a good thing to do.
if (!this.options.db.isOpen()) {
await this.options.db.open()
}
Expand Down Expand Up @@ -171,14 +173,26 @@ export class L1TransportServer extends BaseService<L1TransportServerOptions> {
* TODO: Link to our API spec.
*/
private _registerAllRoutes(): void {
// TODO: Maybe add doc-like comments to each of these routes?

this._registerRoute(
'get',
Copy link
Copy Markdown
Contributor Author

@tynes tynes Apr 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@smartcontracts Any thoughts on how to reduce complexity in this endpoint? We need to add the backend query param to this so that it can determine whether or not its sync'd L1 or L2. Ideally we refactor the db handlers now that we are moving away from confirmed/not confirmed as concepts in this codebase

Copy link
Copy Markdown
Contributor

@annieke annieke Apr 19, 2021

Choose a reason for hiding this comment

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

is backend in this case defaultSource or a different concept?

Copy link
Copy Markdown
Contributor

@gakonst gakonst Apr 21, 2021

Choose a reason for hiding this comment

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

Based on the code, I believe it's the same thing @annieke. @tynes may want to use consistent language and call it backend everywhere.

@tynes what do you think is too complex here? This looks OK to me.

Copy link
Copy Markdown
Contributor Author

@tynes tynes Apr 21, 2021

Choose a reason for hiding this comment

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

what do you think is too complex here? This looks OK to me.

I'm talking about the crazy if/else nesting
https://github.com/ethereum-optimism/optimism/pull/479/files#diff-29a43f33722b902c31ee7224a372262d2b73f170ed0af5be6aca1b54aa505a72L183-L211

'/eth/syncing',
async (): Promise<SyncingResponse> => {
const highestL2BlockNumber = await this.state.db.getHighestL2BlockNumber()
const currentL2Block = await this.state.db.getLatestTransaction()
async (req): Promise<SyncingResponse> => {
const backend = req.query.backend || this.options.defaultBackend

let currentL2Block
let highestL2BlockNumber
switch (backend) {
case 'l1':
currentL2Block = await this.state.db.getLatestTransaction()
highestL2BlockNumber = await this.state.db.getHighestL2BlockNumber()
break
case 'l2':
currentL2Block = await this.state.db.getLatestUnconfirmedTransaction()
highestL2BlockNumber = await this.state.db.getHighestSyncedUnconfirmedBlock()
break
default:
throw new Error(`Unknown transaction backend ${backend}`)
}

if (currentL2Block === null) {
if (highestL2BlockNumber === null) {
Expand Down Expand Up @@ -329,21 +343,19 @@ export class L1TransportServer extends BaseService<L1TransportServerOptions> {
this._registerRoute(
'get',
'/transaction/latest',
async (): Promise<TransactionResponse> => {
let transaction = await this.state.db.getLatestFullTransaction()
if (this.options.showUnconfirmedTransactions) {
const latestUnconfirmedTx = await this.state.db.getLatestUnconfirmedTransaction()
if (
transaction === null ||
transaction === undefined ||
latestUnconfirmedTx.index >= transaction.index
) {
transaction = latestUnconfirmedTx
}
}
async (req): Promise<TransactionResponse> => {
const backend = req.query.backend || this.options.defaultBackend
let transaction = null

if (transaction === null) {
transaction = await this.state.db.getLatestFullTransaction()
switch (backend) {
case 'l1':
transaction = await this.state.db.getLatestFullTransaction()
break
case 'l2':
transaction = await this.state.db.getLatestUnconfirmedTransaction()
break
default:
throw new Error(`Unknown transaction backend ${backend}`)
}

if (transaction === null) {
Expand All @@ -368,17 +380,22 @@ export class L1TransportServer extends BaseService<L1TransportServerOptions> {
'get',
'/transaction/index/:index',
async (req): Promise<TransactionResponse> => {
const backend = req.query.backend || this.options.defaultBackend
let transaction = null
if (this.options.showUnconfirmedTransactions) {
transaction = await this.state.db.getUnconfirmedTransactionByIndex(
BigNumber.from(req.params.index).toNumber()
)
}

if (transaction === null) {
transaction = await this.state.db.getFullTransactionByIndex(
BigNumber.from(req.params.index).toNumber()
)
switch (backend ) {
case 'l1':
transaction = await this.state.db.getFullTransactionByIndex(
BigNumber.from(req.params.index).toNumber()
)
break
case 'l2':
transaction = await this.state.db.getUnconfirmedTransactionByIndex(
BigNumber.from(req.params.index).toNumber()
)
break
default:
throw new Error(`Unknown transaction backend ${backend}`)
}

if (transaction === null) {
Expand Down Expand Up @@ -456,21 +473,19 @@ export class L1TransportServer extends BaseService<L1TransportServerOptions> {
this._registerRoute(
'get',
'/stateroot/latest',
async (): Promise<StateRootResponse> => {
let stateRoot = await this.state.db.getLatestStateRoot()
if (this.options.showUnconfirmedTransactions) {
const latestUnconfirmedStateRoot = await this.state.db.getLatestUnconfirmedStateRoot()
if (
stateRoot === null ||
stateRoot === undefined ||
latestUnconfirmedStateRoot.index >= stateRoot.index
) {
stateRoot = latestUnconfirmedStateRoot
}
}
async (req): Promise<StateRootResponse> => {
const backend = req.query.backend || this.options.defaultBackend
let stateRoot = null

if (stateRoot === null) {
stateRoot = await this.state.db.getLatestStateRoot()
switch (backend) {
case 'l1':
stateRoot = await this.state.db.getLatestStateRoot()
break
case 'l2':
stateRoot = await this.state.db.getLatestUnconfirmedStateRoot()
break
default:
throw new Error(`Unknown transaction backend ${backend}`)
}

if (stateRoot === null) {
Expand All @@ -495,17 +510,22 @@ export class L1TransportServer extends BaseService<L1TransportServerOptions> {
'get',
'/stateroot/index/:index',
async (req): Promise<StateRootResponse> => {
const backend = req.query.backend || this.options.defaultBackend
let stateRoot = null
if (this.options.showUnconfirmedTransactions) {
stateRoot = await this.state.db.getUnconfirmedStateRootByIndex(
BigNumber.from(req.params.index).toNumber()
)
}

if (stateRoot === null) {
stateRoot = await this.state.db.getStateRootByIndex(
BigNumber.from(req.params.index).toNumber()
)
switch (backend) {
case 'l1':
stateRoot = await this.state.db.getStateRootByIndex(
BigNumber.from(req.params.index).toNumber()
)
break
case 'l2':
stateRoot = await this.state.db.getUnconfirmedStateRootByIndex(
BigNumber.from(req.params.index).toNumber()
)
break
default:
throw new Error(`Unknown transaction backend ${backend}`)
}

if (stateRoot === null) {
Expand Down