-
Notifications
You must be signed in to change notification settings - Fork 75
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
soroban-rpc: evict ledger entries #925
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ | |
|
||
import ( | ||
"context" | ||
"crypto/sha256" | ||
"errors" | ||
"fmt" | ||
"sync" | ||
|
@@ -247,6 +248,10 @@ | |
return err | ||
} | ||
|
||
if err := s.evictLedgerEntries(tx, ledgerCloseMeta); err != nil { | ||
return err | ||
} | ||
|
||
if err := s.ingestLedgerCloseMeta(tx, ledgerCloseMeta); err != nil { | ||
return err | ||
} | ||
|
@@ -279,3 +284,43 @@ | |
} | ||
return nil | ||
} | ||
|
||
func (s *Service) evictLedgerEntries(tx db.WriteTx, ledgerCloseMeta xdr.LedgerCloseMeta) error { | ||
if ledgerCloseMeta.V != 2 { | ||
return fmt.Errorf("unexpected close meta version: %d", ledgerCloseMeta.V) | ||
} | ||
keysToEvict := make([]xdr.LedgerKey, len(ledgerCloseMeta.V2.EvictedTemporaryLedgerKeys)+len(ledgerCloseMeta.V2.EvictedPersistentLedgerEntries)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we only need to process evicted temporary ledger keys because the evicted persistent ledger entries are already included in the change reader stream, see stellar/go#5037 I just started working on a similar pr which only handles temporary ledger key eviction https://github.com/stellar/soroban-tools/pull/926/files There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately we can't transparently delete persistent ledger entries though (see TODO comment below) so I will need to detect them anyways. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that you both are correct here, and the solution should be "other". In a similar way to core, we need to find a way ( or maybe not ? ) to help generate the restorefootprint for evicted ledger entries. however, that doesn't need to be handled in the same way. Core are going to store large bloom filters to accomodate that. |
||
l := copy(keysToEvict, ledgerCloseMeta.V2.EvictedTemporaryLedgerKeys) | ||
for i, entry := range ledgerCloseMeta.V2.EvictedPersistentLedgerEntries { | ||
// TODO: we probably shouldn't be deleting evicted persistent ledger entries cold turkey. | ||
// Otherwise preflighting will fail for restoreFootprint. | ||
// For the restoreFootPrint preflighting we need to confirm that the entry existed and its size (for the resource estimation). | ||
// so maybe we should store that. | ||
key, err := entry.LedgerKey() | ||
if err != nil { | ||
return err | ||
} | ||
keysToEvict[l+i] = key | ||
} | ||
for _, key := range keysToEvict { | ||
if err := tx.LedgerEntryWriter().DeleteLedgerEntry(key); err != nil { | ||
return err | ||
} | ||
// Also delete the matching expiration ledger entry | ||
bin, err := key.MarshalBinary() | ||
if err != nil { | ||
return err | ||
} | ||
expirationKey := xdr.LedgerKey{ | ||
Type: xdr.LedgerEntryTypeExpiration, | ||
Expiration: &xdr.LedgerKeyExpiration{ | ||
KeyHash: sha256.Sum256(bin), | ||
}, | ||
} | ||
if err := tx.LedgerEntryWriter().DeleteLedgerEntry(expirationKey); err != nil { | ||
return err | ||
} | ||
|
||
} | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,150 @@ | ||
package test | ||
|
||
import ( | ||
"context" | ||
"crypto/sha256" | ||
"testing" | ||
"time" | ||
|
||
"github.com/creachadair/jrpc2" | ||
"github.com/creachadair/jrpc2/jhttp" | ||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/stellar/go/keypair" | ||
"github.com/stellar/go/txnbuild" | ||
"github.com/stellar/go/xdr" | ||
|
||
"github.com/stellar/soroban-tools/cmd/soroban-rpc/internal/methods" | ||
) | ||
|
||
func TestEvictTemporaryLedgerEntries(t *testing.T) { | ||
test := NewTest(t) | ||
|
||
ch := jhttp.NewChannel(test.sorobanRPCURL(), nil) | ||
client := jrpc2.NewClient(ch, nil) | ||
|
||
sourceAccount := keypair.Root(StandaloneNetworkPassphrase) | ||
address := sourceAccount.Address() | ||
account := txnbuild.NewSimpleAccount(address, 0) | ||
|
||
helloWorldContract := getHelloWorldContract(t) | ||
|
||
params := preflightTransactionParams(t, client, txnbuild.TransactionParams{ | ||
SourceAccount: &account, | ||
IncrementSequenceNum: true, | ||
Operations: []txnbuild.Operation{ | ||
createInstallContractCodeOperation(account.AccountID, helloWorldContract), | ||
}, | ||
BaseFee: txnbuild.MinBaseFee, | ||
Preconditions: txnbuild.Preconditions{ | ||
TimeBounds: txnbuild.NewInfiniteTimeout(), | ||
}, | ||
}) | ||
tx, err := txnbuild.NewTransaction(params) | ||
assert.NoError(t, err) | ||
sendSuccessfulTransaction(t, client, sourceAccount, tx) | ||
|
||
params = preflightTransactionParams(t, client, txnbuild.TransactionParams{ | ||
SourceAccount: &account, | ||
IncrementSequenceNum: true, | ||
Operations: []txnbuild.Operation{ | ||
createCreateContractOperation(t, address, helloWorldContract, StandaloneNetworkPassphrase), | ||
}, | ||
BaseFee: txnbuild.MinBaseFee, | ||
Preconditions: txnbuild.Preconditions{ | ||
TimeBounds: txnbuild.NewInfiniteTimeout(), | ||
}, | ||
}) | ||
tx, err = txnbuild.NewTransaction(params) | ||
assert.NoError(t, err) | ||
sendSuccessfulTransaction(t, client, sourceAccount, tx) | ||
|
||
contractID := getContractID(t, address, testSalt, StandaloneNetworkPassphrase) | ||
invokeIncTemporaryEntryParams := txnbuild.TransactionParams{ | ||
SourceAccount: &account, | ||
IncrementSequenceNum: true, | ||
Operations: []txnbuild.Operation{ | ||
createInvokeHostOperation( | ||
address, | ||
contractID, | ||
"inc_tmp", | ||
), | ||
}, | ||
BaseFee: txnbuild.MinBaseFee, | ||
Preconditions: txnbuild.Preconditions{ | ||
TimeBounds: txnbuild.NewInfiniteTimeout(), | ||
}, | ||
} | ||
params = preflightTransactionParams(t, client, invokeIncTemporaryEntryParams) | ||
tx, err = txnbuild.NewTransaction(params) | ||
assert.NoError(t, err) | ||
sendSuccessfulTransaction(t, client, sourceAccount, tx) | ||
|
||
contractIDHash := xdr.Hash(contractID) | ||
counterSym := xdr.ScSymbol("COUNTER") | ||
key := xdr.LedgerKey{ | ||
Type: xdr.LedgerEntryTypeContractData, | ||
ContractData: &xdr.LedgerKeyContractData{ | ||
Contract: xdr.ScAddress{ | ||
Type: xdr.ScAddressTypeScAddressTypeContract, | ||
ContractId: &contractIDHash, | ||
}, | ||
Key: xdr.ScVal{ | ||
Type: xdr.ScValTypeScvSymbol, | ||
Sym: &counterSym, | ||
}, | ||
Durability: xdr.ContractDataDurabilityTemporary, | ||
}, | ||
} | ||
|
||
// make sure the ledger entry exists and so does the expiration entry counterpart | ||
|
||
keyB64, err := xdr.MarshalBase64(key) | ||
require.NoError(t, err) | ||
|
||
getLedgerEntryRequest := methods.GetLedgerEntryRequest{ | ||
Key: keyB64, | ||
} | ||
var getLedgerEntryResult methods.GetLedgerEntryResponse | ||
err = client.CallResult(context.Background(), "getLedgerEntry", getLedgerEntryRequest, &getLedgerEntryResult) | ||
require.NoError(t, err) | ||
|
||
binKey, err := key.MarshalBinary() | ||
assert.NoError(t, err) | ||
|
||
expirationKey := xdr.LedgerKey{ | ||
Type: xdr.LedgerEntryTypeExpiration, | ||
Expiration: &xdr.LedgerKeyExpiration{ | ||
KeyHash: sha256.Sum256(binKey), | ||
}, | ||
} | ||
|
||
keyB64, err = xdr.MarshalBase64(expirationKey) | ||
require.NoError(t, err) | ||
getExpirationLedgerEntryRequest := methods.GetLedgerEntryRequest{ | ||
Key: keyB64, | ||
} | ||
|
||
err = client.CallResult(context.Background(), "getLedgerEntry", getExpirationLedgerEntryRequest, &getLedgerEntryResult) | ||
assert.NoError(t, err) | ||
|
||
// Wait until the entry gets evicted | ||
evicted := false | ||
for i := 0; i < 5000; i++ { | ||
err = client.CallResult(context.Background(), "getLedgerEntry", getLedgerEntryRequest, &getLedgerEntryResult) | ||
if err != nil { | ||
evicted = true | ||
t.Logf("ledger entry evicted") | ||
break | ||
} | ||
t.Log("waiting for ledger entry to get evicted") | ||
time.Sleep(time.Second) | ||
} | ||
|
||
require.True(t, evicted) | ||
|
||
// Make sure that the expiration ledger entry was also evicted | ||
err = client.CallResult(context.Background(), "getLedgerEntry", getExpirationLedgerEntryRequest, &getLedgerEntryResult) | ||
assert.Error(t, err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be great if this change to the counter would be to a completely new key ( i.e. "TMP_COUNTER" ), so that we can be sure that we're testing the right thing.