Skip to content

Commit

Permalink
fix(cosmic-swingset): add exportCallback interlock
Browse files Browse the repository at this point in the history
The swingstore export-data callback gives us export-data records,
which must be written into IAVL by sending them over to the golang
side with swingStoreExportCallback . However, that callback isn't
ready right away, so if e.g. openSwingStore() were to invoke it, we
might lose those records. Likewise saveOutsideState() gathers the
chainSends just before calling commit, so if the callback were invoked
during commit(), those records would be left for a subsequent block,
which would break consensus if the node crashed before the next
commit.

This commit adds an `allowExportCallback` flag, to catch these two
cases. It is enabled at the start of AG_COSMOS_INIT and BEGIN_BLOCK,
and then disabled just before we flush the chain sends in
saveOutsideState() (called during COMMIT_BLOCK).

Note that swingstore is within its rights to call exportCallback
during openSwingStore() or commit(), it just happens to not do so
right now. If that changes under maintenance, this guard should
turn a corruption bug into a crash bug

refs #9655
  • Loading branch information
warner committed Aug 12, 2024
1 parent c769606 commit 6547c83
Showing 1 changed file with 23 additions and 0 deletions.
23 changes: 23 additions & 0 deletions packages/cosmic-swingset/src/launch-chain.js
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,25 @@ export async function launch({
}) {
console.info('Launching SwingSet kernel');

// The swingstore export-data callback gives us export-data records,
// which must be written into IAVL by sending them over to the
// golang side with swingStoreExportCallback . However, that
// callback isn't ready right away, so if e.g. openSwingStore() were
// to invoke it, we might lose those records. Likewise
// saveOutsideState() gathers the chainSends just before calling
// commit, so if the callback were invoked during commit(), those
// records would be left for a subsequent block, which would break
// consensus if the node crashed before the next commit. So this
// `allowExportCallback` flag serves to catch these two cases.
//
// Note that swingstore is within its rights to call exportCallback
// during openSwingStore() or commit(), it just happens to not do so
// right now. If that changes under maintenance, this guard should
// turn a corruption bug into a crash bug. See
// https://github.com/Agoric/agoric-sdk/issues/9655 for details

let allowExportCallback = false;

// The swingStore's exportCallback is synchronous, however we allow the
// callback provided to launch-chain to be asynchronous. The callbacks are
// invoked sequentially like if they were awaited, and the block manager
Expand All @@ -343,6 +362,7 @@ export async function launch({
const swingStoreExportSyncCallback =
swingStoreExportCallback &&
(updates => {
assert(allowExportCallback, 'export-data callback called at bad time');
pendingSwingStoreExport = swingStoreExportCallbackWithQueue(updates);
});

Expand Down Expand Up @@ -470,6 +490,7 @@ export async function launch({
}

async function saveOutsideState(blockHeight) {
allowExportCallback = false;
const chainSends = await clearChainSends();
kvStore.set(getHostKey('height'), `${blockHeight}`);
kvStore.set(getHostKey('chainSends'), JSON.stringify(chainSends));
Expand Down Expand Up @@ -889,6 +910,7 @@ export async function launch({
// );
switch (action.type) {
case ActionType.AG_COSMOS_INIT: {
allowExportCallback = true; // cleared by saveOutsideState in COMMIT_BLOCK
const { blockHeight, isBootstrap, upgradeDetails } = action;

if (!blockNeedsExecution(blockHeight)) {
Expand Down Expand Up @@ -975,6 +997,7 @@ export async function launch({
}

case ActionType.BEGIN_BLOCK: {
allowExportCallback = true; // cleared by saveOutsideState in COMMIT_BLOCK
const { blockHeight, blockTime, params } = action;
blockParams = parseParams(params);
verboseBlocks &&
Expand Down

0 comments on commit 6547c83

Please sign in to comment.