Skip to content

Fix for Mempool and BP using different State Services #243

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
rpanic merged 2 commits into from
Jan 31, 2025
Merged

Fix for Mempool and BP using different State Services #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73b4659
Change to use same instance
ejMina226 Dec 13, 2024
c2eb14c
Update test
ejMina226 Dec 13, 2024
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
6 changes: 2 additions & 4 deletions packages/common/src/config/ModuleContainer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
DependencyContainer,
Frequency,
InjectionToken,
instancePerContainerCachingFactory,
instanceCachingFactory,
isClassProvider,
isFactoryProvider,
isTokenProvider,
Expand Down Expand Up @@ -404,9 +404,7 @@ export class ModuleContainer<
// this enables us to have a singletoned factory
// that returns the same instance for each resolve
this.container.register(key, {
useFactory: instancePerContainerCachingFactory(
declaration.useFactory
),
useFactory: instanceCachingFactory(declaration.useFactory),
});
} else if (isClassProvider(declaration)) {
this.container.register(key, declaration, {
Expand Down
101 changes: 101 additions & 0 deletions packages/sequencer/test/integration/BlockProduction.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,107 @@ describe("block production", () => {
expect(newState).toBeUndefined();
}, 30_000);

it("should produce txs in non-consecutive blocks", async () => {
Copy link
Member

@rpanic rpanic Dec 16, 2024

Choose a reason for hiding this comment

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

@ejMina226 What is the unique thing of this test that isn't covered in the others? Can this be modeled using the multiple blocks test in line 471(ish)?

Copy link
Contributor Author

@ejMina226 ejMina226 Dec 16, 2024

Choose a reason for hiding this comment

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

I wanted the txs to appear in non-consecutive blocks.

Copy link
Contributor Author

@ejMina226 ejMina226 Dec 16, 2024

Choose a reason for hiding this comment

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

The should produce multiple blocks with multiple batches with multiple transactions has consecutive blocks of txs, I think.

Copy link
Contributor Author

@ejMina226 ejMina226 Dec 16, 2024

Choose a reason for hiding this comment

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

Happy to remove if you think it overkill.

Copy link
Member

@rpanic rpanic Dec 16, 2024

Choose a reason for hiding this comment

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

What do you mean by non-consecutive? If I understand it correctly, you have 1 tx per block here

Copy link
Contributor Author

@ejMina226 ejMina226 Dec 16, 2024

Choose a reason for hiding this comment

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

There are two different senders. I wanted the first sender to have their txs in non-consecutive blocks. The second sender is just to ensure the interim blocks are non-empty.

log.setLevel("TRACE");

const privateKey = PrivateKey.random();
const publicKey = privateKey.toPublicKey();

const privateKey2 = PrivateKey.random();
const publicKey2 = privateKey2.toPublicKey();

await mempool.add(
createTransaction({
runtime,
method: ["Balance", "setBalanceIf"],
privateKey,
args: [publicKey, UInt64.from(100), Bool(true)],
nonce: 0,
})
);

// let [block, batch] = await blockTrigger.produceBlockAndBatch();
const block = await blockTrigger.produceBlock();

expect(block).toBeDefined();

expect(block!.transactions).toHaveLength(1);
expect(block!.transactions[0].status.toBoolean()).toBe(true);
expect(block!.transactions[0].statusMessage).toBeUndefined();

expect(block!.transactions[0].stateTransitions).toHaveLength(1);
expect(block!.transactions[0].protocolTransitions).toHaveLength(2);

await blockTrigger.produceBlock();

await mempool.add(
createTransaction({
runtime,
method: ["Balance", "setBalanceIf"],
privateKey: privateKey2,
args: [publicKey2, UInt64.from(100), Bool(true)],
nonce: 0,
})
);
await blockTrigger.produceBlock();

await mempool.add(
createTransaction({
runtime,
method: ["Balance", "setBalanceIf"],
privateKey: privateKey2,
args: [publicKey2, UInt64.from(100), Bool(true)],
nonce: 1,
})
);
await blockTrigger.produceBlock();

await mempool.add(
createTransaction({
runtime,
method: ["Balance", "setBalanceIf"],
privateKey: privateKey2,
args: [publicKey2, UInt64.from(100), Bool(true)],
nonce: 2,
})
);
await blockTrigger.produceBlock();

await mempool.add(
createTransaction({
runtime,
method: ["Balance", "setBalanceIf"],
privateKey: privateKey2,
args: [publicKey2, UInt64.from(100), Bool(true)],
nonce: 3,
})
);
await blockTrigger.produceBlock();

// Second tx
await mempool.add(
createTransaction({
runtime,
method: ["Balance", "setBalanceIf"],
privateKey,
args: [publicKey, UInt64.from(100), Bool(true)],
nonce: 1,
})
);

log.info("Starting second block");

const block2 = await blockTrigger.produceBlock();

expect(block2).toBeDefined();

expect(block2!.transactions).toHaveLength(1);
console.log(block2!.transactions[0]);
console.log(block2!.transactions[0].statusMessage);
expect(block2!.transactions[0].status.toBoolean()).toBe(true);
expect(block2!.transactions[0].statusMessage).toBeUndefined();
}, 60_000);

const numberTxs = 3;

it("should produce block with multiple transaction", async () => {
Expand Down
Loading