Propose traced transaction construction #17796

ryoqun · 2021-06-07T15:43:48Z

(boring meta-joke due to imminent sleep time.)

Problem

This is half-done!

Summary of Changes

Push more commitz and squaz

context (original idea is written when the itoken reviewing: #15927 (comment))

ryoqun · 2021-06-07T15:57:44Z

docs/src/proposals/traced-transaction-construction.md

+
+## Problem
+
+With more composition comes in the Solana's program ecosystem, it will be harder


this doc is still in the draft phase. but I think the first half of this is written decently. ;)

ryoqun · 2021-06-07T16:06:04Z

docs/src/proposals/traced-transaction-construction.md

+
+## Future possible expansions
+
+- Wallet account balance delta check: append VerifyTokenBalances ix at last and pass the expected balances of spl-tokens or hash of it (also with delegation status).


this is my (hopefully nice) solution to this: #17762

also, dynamic ix append as directed traced simulation might also be useful.

ryoqun · 2021-06-07T16:10:02Z

docs/src/proposals/traced-transaction-construction.md

+## Future possible expansions
+
+- Wallet account balance delta check: append VerifyTokenBalances ix at last and pass the expected balances of spl-tokens or hash of it (also with delegation status).
+- Browser-side bpf program simualtion for server-less architecture with online wasm translation


this is only possible because solana tx execution doesn't touch the whole account state in the chain. :)

ryoqun · 2021-06-07T16:23:11Z

docs/src/proposals/traced-transaction-construction.md

+This reduces the number of required accounts for CPI and clients from 7 to 3, reducing by 4.
+(Obviously, we can't actually deploy these changes because this will break
+compatibility. This is only for illustration of effective applicability to
+moderately-complex program construct.)


also, maybe worth to mention that this doesn't reduce the resultant tx binary size, we need still this badly for more composability: #15927 cc: @jstarry

The good thing is that this nor the page-indexed tx block each other. :)

ryoqun · 2021-06-07T16:25:19Z

docs/src/proposals/traced-transaction-construction.md

+
+prevent program  detect to be simulated or really-executed
+
+


also, write about the strip of is_signer and no use case limited by this stripping.

jstarry · 2021-06-07T17:25:23Z

docs/src/proposals/traced-transaction-construction.md

+deployed programs can be now upgradeable. This severely hampers ongoing
+consumed-program's implementation refinement in the areas of data migration and
+improved concurrency, by preventing transparent account switching and
+sharding with vectorized accounts, respectively.


what is meant by "vectorized accounts"?

@jstarry don't be bothered with it.. ;) well, I used this fancy adjective, remotely referencing the usage like vectorization with SSE4/AVX2. Maybe sharding by itself should be enough.

Sharding here means avoiding write lock to the same internal account by using different accounts to write by deriving from the first byte of other account addresses like order_id for DEX. So, program can avoid write lock contention easily. Also, see the sharding example for code.

I want to say, hiding internal account addresses in this way makes these optimizations can be introduced by drop-in basis.

Ah got it, I like the idea of abstracting away implementation details like this, thanks for the explanation!

ryoqun · 2021-06-12T14:40:42Z

docs/src/proposals/traced-transaction-construction.md

+can be traced:
+
+```rust
+fn access_as_unsigned(address: &Pubkey, flags = enum { READ, WRITE}): 


Considering this #17901,

Maybe we should augment this by returning enum {AccountInfo, AccountRef} with new flag ACCESS_AS_REF. so that we can avoid any serializing into program execution environment.

AccountRef can only be used for passing to child cpi.

jackcmay · 2021-06-14T06:21:28Z

docs/src/proposals/traced-transaction-construction.md

+        program_id,
+    );
+   let transfer_counter = access_as_unsigned(counter_address, READ_WRITE)?;
+   let clock_sysvar = access_as_unsigned(sysvars::clock::id(), READ_WRITE)?;


Why is clock rw?

jackcmay · 2021-06-14T06:22:51Z

docs/src/proposals/traced-transaction-construction.md

+        ],
+        program_id,
+    );
+   let transfer_counter = access_as_unsigned(counter_address, READ_WRITE)?;


What type is transfer_counter?

jackcmay · 2021-06-14T06:32:07Z

This is interesting. I like that it reduces and simplifies a program's calling interface by hiding internal-only accounts that the caller doesn't need to be or shouldn't be aware of.

jackcmay · 2021-06-14T06:47:11Z

In addition to a syscall used to discover dependent accounts, if any cpi is invoked during simulation that specifies new accounts, those new accounts should also be added to the list. During non-simulation those dependent accounts would be listed in the tx, loaded along with the rest of the accounts and referenced as needed.

stale · 2021-06-22T16:11:59Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale · 2021-07-01T19:51:26Z

This stale pull request has been automatically closed. Thank you for your contributions.

ryoqun · 2021-07-14T11:31:26Z

docs/src/proposals/traced-transaction-construction.md

+@@ -22,13 +22,12 @@ pub fn process_instruction(
+     let account_info_iter = &mut accounts.iter();
+
+     let funder_info = next_account_info(account_info_iter)?;


related to #18588

ryoqun · 2021-07-14T11:55:31Z

docs/src/proposals/traced-transaction-construction.md

@@ -0,0 +1,247 @@
+# Traced transaction construction via RPC's `simulateTransaction`


traced => instrumented?

stale · 2021-07-21T12:26:58Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

ruuda · 2021-07-21T13:46:15Z

Tracing can only reveal references in code that is actually executed. Consider something like

let clock = Clock::get()?;
if clock.slot % 2 == 0 {
    reference_account_a();
} else {
    reference_account_b();
}

If the RPC traces the program execution and says it referenced account A, then it is likely that by the time it executes for real, the program will try to reference account B.

This is similar to how tests can only reveal bugs in code that is executed; to get an exhaustive answer, you need some kind of (static) program analysis or abstract interpretation.

ryoqun · 2021-07-22T16:47:44Z

Tracing can only reveal references in code that is actually executed. Consider something like
let clock = Clock::get()?;
if clock.slot % 2 == 0 {
    reference_account_a();
} else {
    reference_account_b();
}
If the RPC traces the program execution and says it referenced account A, then it is likely that by the time it executes for real, the program will try to reference account B.

This is similar to how tests can only reveal bugs in code that is executed; to get an exhaustive answer, you need some kind of (static) program analysis or abstract interpretation.

@ruuda thanks for chiming in! you're first penguin from community. I really appreciate that.

Yeah, I understand the limitation. You have a point. But I still think this proposal could be reasonable solution for the problems described, at the cost of the strict requirement of rpc's simulation when issuing transactions to any advanced programs.

Maybe I should have added a prose like this?:

# Limitations (or Compromises)

This facility must not be abused and used wisely, needing proper developer
guidelines/communication. Firstly, dynamic account selection logic shouldn't
be timing sensitive or not change frequently. Because there is unavoidable
timing delays between simulation and actual execution on chain, transactions
could fail unexpectedly if implemented badly.

In other words, while dynamic account loading could give greater flexibility, the
logic should rather be static over time and for individual user sessions. The dynamic loading
isn't goal par itself. Rather this proposal tries to reduce and simplify a program's
calling interface by hiding internal-only accounts that the caller doesn't need to be or
shouldn't be aware of.

# Alternatives

- Some (static) IDL/DSL for account loading logic: hard to develop/design to cover any conceivable use cases. extra complexity
- provide no programmatic/systemic way to describe account loading (status quo): hard to compose defi programs.

How do you like?

ryoqun · 2021-07-22T16:51:43Z

docs/src/proposals/traced-transaction-construction.md

+
+race condition around program deploy (= possible account loading change) => just retry also seek to the ix versioning for perfection
+
+(traced program just load dummy addresses for tx versionsing for completeness)


also, I have to embed this pr comment into doc: #17796 (comment)

ruuda · 2021-07-23T08:43:01Z

How do you like?

I don’t have a strong opinion on it, I just wanted to point out the limitation. Personally I think that with the way the Solana runtime is exposed to Rust at the moment, Rust is not a great language for writing Solana programs, and in the long run I expect some DSL to take over. A DSL could take care of those details internally, with a compiler figuring out which accounts are referenced.

stale · 2021-07-30T09:25:46Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale · 2021-08-10T03:17:13Z

This stale pull request has been automatically closed. Thank you for your contributions.

stale · 2021-08-28T16:50:58Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

stale · 2021-09-04T23:46:56Z

This stale pull request has been automatically closed. Thank you for your contributions.

stale · 2021-09-19T19:32:41Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

ryoqun added 9 commits June 7, 2021 23:19

Propose traced transaction construction

2025d84

Tweak

41c9057

Tweak

89efda3

Tweak

2580ddd

Tweak

3b926fd

Tweak

fc2e411

Tweak

cd2fc6d

Tweak

ed92de0

Tweak

75516cc

This was referenced Jun 7, 2021

Add proposal for interest bearing tokens #15927

Closed

Add proposal for transactions v2 and address map program #17103

Merged

ryoqun commented Jun 7, 2021

View reviewed changes

jstarry reviewed Jun 7, 2021

View reviewed changes

ryoqun mentioned this pull request Jun 12, 2021

Amend transactions v2 proposal with dependent accounts #17901

Closed

ryoqun commented Jun 12, 2021

View reviewed changes

jackcmay reviewed Jun 14, 2021

View reviewed changes

stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jun 22, 2021

stale bot closed this Jul 1, 2021

ryoqun reopened this Jul 12, 2021

stale bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Jul 12, 2021

ryoqun mentioned this pull request Jul 12, 2021

Deterministic addressing for serum markets project-serum/serum-dex#122

Open

ryoqun mentioned this pull request Jul 14, 2021

Provide transaction fee payer and system program via sysvars #18588

Closed

ryoqun commented Jul 14, 2021

View reviewed changes

ryoqun mentioned this pull request Jul 14, 2021

would it provides balance changes response in rpc, simulateTransaction ? #18509

Closed

ryoqun commented Jul 14, 2021

View reviewed changes

jon-chuang mentioned this pull request Jul 17, 2021

RPC simulations should allow for passing in user provided account data #18746

Closed

stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jul 21, 2021

stale bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Jul 21, 2021

ryoqun commented Jul 22, 2021

View reviewed changes

stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Jul 30, 2021

stale bot closed this Aug 10, 2021

ryoqun mentioned this pull request Aug 13, 2021

ABI v2 Proposal #19191

Closed

ryoqun reopened this Aug 18, 2021

stale bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Aug 18, 2021

stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Aug 28, 2021

stale bot closed this Sep 4, 2021

ryoqun reopened this Sep 7, 2021

stale bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Sep 7, 2021

ryoqun mentioned this pull request Sep 7, 2021

Bad actors can abuse the privilege extension feature for Cross-Program Invocations via system_instruction::transfer, spl_token::instruction::approve, spl_token::instruction::transfer #17762

Closed

stale bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Sep 19, 2021

mvines closed this Feb 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propose traced transaction construction #17796

Propose traced transaction construction #17796

ryoqun commented Jun 7, 2021 •

edited

Loading

ryoqun Jun 7, 2021

ryoqun Jun 7, 2021

ryoqun Jun 7, 2021

ryoqun Jun 7, 2021

ryoqun Jun 7, 2021 •

edited

Loading

ryoqun Jun 7, 2021

jstarry Jun 7, 2021

ryoqun Jun 9, 2021

jstarry Jun 9, 2021

ryoqun Jun 12, 2021 •

edited

Loading

jackcmay Jun 14, 2021

jackcmay Jun 14, 2021

jackcmay commented Jun 14, 2021

jackcmay commented Jun 14, 2021

stale bot commented Jun 22, 2021

stale bot commented Jul 1, 2021

ryoqun Jul 14, 2021

ryoqun Jul 14, 2021

stale bot commented Jul 21, 2021

ruuda commented Jul 21, 2021

ryoqun commented Jul 22, 2021

ryoqun Jul 22, 2021

ruuda commented Jul 23, 2021

stale bot commented Jul 30, 2021

stale bot commented Aug 10, 2021

stale bot commented Aug 28, 2021

stale bot commented Sep 4, 2021

stale bot commented Sep 19, 2021


		## Problem

		With more composition comes in the Solana's program ecosystem, it will be harder


		## Future possible expansions

		- Wallet account balance delta check: append VerifyTokenBalances ix at last and pass the expected balances of spl-tokens or hash of it (also with delegation status).

		@@ -0,0 +1,247 @@
		# Traced transaction construction via RPC's `simulateTransaction`


		race condition around program deploy (= possible account loading change) => just retry also seek to the ix versioning for perfection

		(traced program just load dummy addresses for tx versionsing for completeness)

Propose traced transaction construction #17796

Propose traced transaction construction #17796

Conversation

ryoqun commented Jun 7, 2021 • edited Loading

Problem

Summary of Changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryoqun Jun 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ryoqun Jun 12, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jackcmay commented Jun 14, 2021

jackcmay commented Jun 14, 2021

stale bot commented Jun 22, 2021

stale bot commented Jul 1, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stale bot commented Jul 21, 2021

ruuda commented Jul 21, 2021

ryoqun commented Jul 22, 2021

Choose a reason for hiding this comment

ruuda commented Jul 23, 2021

stale bot commented Jul 30, 2021

stale bot commented Aug 10, 2021

stale bot commented Aug 28, 2021

stale bot commented Sep 4, 2021

stale bot commented Sep 19, 2021

ryoqun commented Jun 7, 2021 •

edited

Loading

ryoqun Jun 7, 2021 •

edited

Loading

ryoqun Jun 12, 2021 •

edited

Loading