-
Notifications
You must be signed in to change notification settings - Fork 157
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
add guide for extending a persistent entry and contract using the javascript sdk #1163
base: main
Are you sure you want to change the base?
add guide for extending a persistent entry and contract using the javascript sdk #1163
Conversation
Heya @Mackenzie-OO7! Thank you for another great submission. Apologies for the delay in review, we'll get to this ASAP! |
Hi @Mackenzie-OO7 - thanks again so much for your contributions! Have you been able to look at the feedback yet? |
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.
Please review:
https://developers.stellar.org/docs/learn/encyclopedia/storage/state-archival#extendfootprintttlop
Operation.extendFootprintTtl The Read-only set needs to contain the ledger key and the read/write section needs to be empty
ExtendFootprintTTLOp is a Soroban operation that will extend the live until ledger of the entries specified in the read-only set of the footprint. The read-write set must be empty.
hi! somehow I missed this, but I see there's new feedback now and I'll get to it. thanks for the reminder. |
apologies for the delay, I had some issues with my machine, but I’ve sorted that out now. |
Hi @Mackenzie-OO7! Just checking, is this ready for re-review? |
Yes it is |
}) | ||
.addOperation( | ||
StellarSdk.Operation.extendFootprintTtl({ | ||
extendTo: 500_000, //The number of ledgers past the LCL (last closed ledger) by which to extend |
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 recommend clarifying how long of an extension this is in "human time" terms in the comment - 500K ledgers at ~5s each is almost a month. Is that intended? Seems high.
|
||
// Attach Soroban transaction data | ||
const sorobanData = new StellarSdk.SorobanDataBuilder() | ||
.setFootprint(footprint) |
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'd recommend removing lines 40-44 and just doing
.setFootprint(footprint) | |
.setReadOnly(persistentEntry) |
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.
And actually this should be passed to setSorobanData
rather than set as a member here. So in summary,
- remove the lines building
footprint
- don't pass
footprint
to the builder options - pass
sorobanData
instead as you're doing w/SorobanDataBuilder
- only then call
.build()
async function extendPersistentEntryTTL(contractId, storageKey, sourceKeypair) { | ||
const server = new Server("https://soroban-testnet.stellar.org"); | ||
const account = await server.getAccount(sourceKeypair.publicKey()); | ||
const fee = "200100"; // 200_000 covers Soroban overhead + a BASE_FEE of 100 stroops |
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.
Simulation will cover assigning the right fee as part of prepareTransaction
, so you can just make this the BASE_FEE
for inclusion into the ledger.
closes #610